2004-01-22 03:48:30

by Valdis Klētnieks

[permalink] [raw]
Subject: 2.6.1-mm5 versus gcc 3.5 snapshot

Is the Fedora GCC 3.5 snapshot on crack, or is this a yet-unfixed not-ready-for 3.5?

gcc-ssa (GCC) 3.5-tree-ssa 20040115 (Fedora Core Rawhide 3.5ssa-108)

produces tons of these:

include/asm/rwsem.h: In function `__down_read_trylock':
include/asm/rwsem.h:126: warning: read-write constraint does not allow a register


Attachments:
(No filename) (226.00 B)

2004-01-22 04:51:19

by Linus Torvalds

[permalink] [raw]
Subject: Re: 2.6.1-mm5 versus gcc 3.5 snapshot



On Wed, 21 Jan 2004 [email protected] wrote:
>
> Is the Fedora GCC 3.5 snapshot on crack, or is this a yet-unfixed not-ready-for 3.5?
>
> gcc-ssa (GCC) 3.5-tree-ssa 20040115 (Fedora Core Rawhide 3.5ssa-108)
>
> produces tons of these:
>
> include/asm/rwsem.h: In function `__down_read_trylock':
> include/asm/rwsem.h:126: warning: read-write constraint does not allow a register

To me that says "compiler on crack". I don't see why a register couldn't
be rw. After all, it clearly can be read, and written to, and gcc does
explicitly mention the '&' modifier in the documentation.

But maybe Richard has some input on what has happened, and can explain the
compiler side of it.. Richard?

Linus

2004-01-22 06:03:05

by Richard Henderson

[permalink] [raw]
Subject: Re: 2.6.1-mm5 versus gcc 3.5 snapshot

On Wed, Jan 21, 2004 at 08:51:14PM -0800, Linus Torvalds wrote:
> > include/asm/rwsem.h:126: warning: read-write constraint does not allow
> > a register
>
> To me that says "compiler on crack". I don't see why a register couldn't
> be rw. After all, it clearly can be read, and written to, and gcc does
> explicitly mention the '&' modifier in the documentation.
>
> But maybe Richard has some input on what has happened, and can explain the
> compiler side of it.. Richard?

You're reading that wrong way-round. It's "+m" and "=m"/"0" that's
disallowed. I.e. if you have matching constraints (or read-write
constrants, which are exactly short-hand for matching constraints),
then you *must* have a register alternative. I.e. you'll get this
warning if you *only* allow memories.

The problem is partially conceptual -- what in the world does

"=m"(x) : "0"(y)

mean? Logically, this makes no sense. The only way it can be resolved
is to create a new memory, copy y in, and copy x out. But that violates
the lvalue promises we've made that make memory constraints useful for
atomic operations.

Partially it's implementation -- if you write

"=m"(x) : "0"(x)

it *requires* that the optimizer be run and that it *must* identify the
common sub-expression. Failure to do so means that the compiler has to
assume we have the x/y situation above, which of course results in a
diagnostic.

Obviously such a computational requirement is impossible with arbitrarily
complex expressions, so that begs the question of "how complex is too
complex". Drawing an arbitrary line that you can explain to users is
impossible. It is easier to simply disallow it entirely.

Finally, the whole thing is pointless. Given the lvalue semantics, you
get *exactly* the same effect from

"=m"(x) : "m"(x)

Since this works for any version of gcc, at any optimization level,
on any arbitrarily complex expression, we strongly recommend (ahem)
that code be modified to use this form.


r~

2004-01-22 15:28:05

by Linus Torvalds

[permalink] [raw]
Subject: Re: 2.6.1-mm5 versus gcc 3.5 snapshot



On Wed, 21 Jan 2004, Richard Henderson wrote:
>
> You're reading that wrong way-round. It's "+m" and "=m"/"0" that's
> disallowed.

Ok, but...

> I.e. if you have matching constraints (or read-write
> constrants, which are exactly short-hand for matching constraints),
> then you *must* have a register alternative. I.e. you'll get this
> warning if you *only* allow memories.
>
> The problem is partially conceptual -- what in the world does
>
> "=m"(x) : "0"(y)

I agree about the latter one, but "+m" (which is what the kernel uses) has
well-defined meaning, and the compiler would be/is silly to complain about
it.

So your arguments fall down flat. If it was

"=m" (x) : "0" (y)

I'd agree with you, but that's not the code the compiler complains about.

Shorthand or not, the "+m" usage is (a) totally logical and (b)
historically allowed.

Please fix the compiler.

Linus

2004-01-22 23:30:21

by Richard Henderson

[permalink] [raw]
Subject: Re: 2.6.1-mm5 versus gcc 3.5 snapshot

On Thu, Jan 22, 2004 at 07:27:52AM -0800, Linus Torvalds wrote:
> Shorthand or not, the "+m" usage is (a) totally logical

Logical or not, "+" is not how reload works; this must be split to use "0".

> and (b) historically allowed.

Allowed (since 2.8 or so), but it didn't always work. The nth bug report
is what prompted the addition of the warning.

> Please fix the compiler.

Maybe someday, but not I'm not rewriting reload today. Given there *is*
an alternative way to write this, it is definitely not a priority.


r~

2004-01-22 23:39:50

by Linus Torvalds

[permalink] [raw]
Subject: Re: 2.6.1-mm5 versus gcc 3.5 snapshot



On Thu, 22 Jan 2004, Richard Henderson wrote:
> On Thu, Jan 22, 2004 at 07:27:52AM -0800, Linus Torvalds wrote:
> > Shorthand or not, the "+m" usage is (a) totally logical
>
> Logical or not, "+" is not how reload works; this must be split to use "0".

Why don't you split it to do "m" instead?

> > Please fix the compiler.
>
> Maybe someday, but not I'm not rewriting reload today. Given there *is*
> an alternative way to write this, it is definitely not a priority.

The point being:
- it's documented
- it is used
- you don't have to fix reload, just the splitting

So why break it? Just do the alternative as the split, since you say it is
equivalent anyway.

Linus

2004-01-23 00:25:59

by Richard Henderson

[permalink] [raw]
Subject: Re: 2.6.1-mm5 versus gcc 3.5 snapshot

On Thu, Jan 22, 2004 at 03:39:44PM -0800, Linus Torvalds wrote:
> > Logical or not, "+" is not how reload works; this must be split to use "0".
>
> Why don't you split it to do "m" instead?

Um, duh. That I can do.

> So why break it?

We didn't break anything, or even change the code involved.
Just added a warning.



r~