Date:	Wed, 13 May 2015 10:50:55 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Louis Langholtz <lou_langholtz@...com>
Cc:	Ingo Molnar <mingo@...nel.org>,
	Valdis Kletnieks <Valdis.Kletnieks@...edu>,
	Peter Anvin <hpa@...or.com>, Brian Gerst <brgerst@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Borislav Petkov <bp@...en8.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Andy Lutomirski <luto@...capital.net>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [tip:x86/build] x86/build: Remove -Wno-sign-compare

On Wed, May 13, 2015 at 10:33 AM, Louis Langholtz <lou_langholtz@...com> wrote:
>
> Are warnings about sign-comparisons an architecture specific issue?

No, I think it's a combination of

 (a) some architectures don't end up caring too much about warnings (I
guess you could call this an "architecture specific issue")

 (b) the real problems tend to be in gcc versions that do f*cking
insane things and warn for code that cannot possibly be written better
any other way. Some architectures use a very constrained set of
compilers, and their set may not have the particular problem.

The classic example of (b) is the whole "comparison with a constant".
Gcc has gotten this wrong so many times that it's not even funny.

The fact is, we often use constants with the "wrong" sign. Tough. Part
of it is that signed constants are simply the only sane case, even if
you then compare against unsigned variables. A compiler that complains
about that is a shit compiler. It's that simple.

This is why "-Wno-sign-compare" was added in the first place. People
who think that you should always compare unsigned variables against
unsigned constants are simply wrong.

And yes, we have a lot of them inside macros. We often use macros as a
way to do generics in C, so code like this:

    #define percpu_add_op(var, val)                                         \
    do {                                                                    \
        ...
            const int pao_ID__ = (__builtin_constant_p(val) &&              \
                                  ((val) == 1 || (val) == -1)) ?            \

where we do magic things if "val" is 1 or -1 (generating "inc" and
"dec" respectively) is not insane. But because this is a generic
thing, sometimes we pass unsigned things around, and you get that
"compare 'unsigned' val against '-1'" thing. Tough.

Sure, we can add random casts in these things. In this particular
example, we could do things like cast both (val) and -1 to
"typeof(var)" and it wouldn't be "wrong".  After all, it's really just
meant for a heuristic (in this case it's a "let's see if this case of
addition is a trivial special case of adding -1, and we'll turn it
into a smaller instruction").

So those kinds of cast things don't actually improve the code, and in
many cases they actively make it less obvious. They aren't worth it.

So it's a tradeoff. Which one is better: get rid of stupid warnings by
using "-Wno-sign-compare" or by polluting the source code with
pointless type casts?

If -Wno-sign-compare had higher quality, I'd be perfectly willing to
add a few pointless casts. But the problem with the gcc sign-compare
warning is that it has traditionally (and apparenrly _still_) had the
quality of undiluted horse-shit.

Which makes it an easy decision to make: "-Wno-sign-compare" is the
right solution. Shut up the crap warnings, without making the source
worse.

                        Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists