[PATCH] get rid of "+m" constraint in i386 rwsems

[PATCH] get rid of "+m" constraint in i386 rwsems

[David Howells]

Hi Linus, Andrew,

Here's a patch to remove the usage of a "+m" constraint in the i386 optimised
rwsem implementation.

David


--- ./include/asm-i386/rwsem.h.orig     2004-05-06 12:54:38.000000000 +0100
+++ ./include/asm-i386/rwsem.h  2004-05-06 12:55:09.000000000 +0100
@@ -134,8 +134,8 @@ LOCK_PREFIX "  cmpxchgl  %2,%0\n\t"
                "  jnz       1b\n\t"
                "2:\n\t"
                "# ending __down_read_trylock\n\t"
-               : "+m"(sem->count), "=&a"(result), "=&r"(tmp)
-               : "i"(RWSEM_ACTIVE_READ_BIAS)
+               : "=m"(sem->count), "=&a"(result), "=&r"(tmp)
+               : "i"(RWSEM_ACTIVE_READ_BIAS), "m"(sem->count)
                : "memory", "cc");
        return result>=0 ? 1 : 0;
 }

Re: [PATCH] get rid of "+m" constraint in i386 rwsems

[Russell King]

On Thu, May 06, 2004 at 12:58:53PM +0100, David Howells wrote:
> Here's a patch to remove the usage of a "+m" constraint in the i386 optimised
> rwsem implementation.

Doesn't the assembly assume that %0 is the same as %4, though because
they're memory operands, the chances of them not being so is pretty
slim?  From the gcc manual, it appears that this may not always be
the case:

|    The ordinary output operands must be write-only; GCC will assume that
| the values in these operands before the instruction are dead and need
| not be generated.  Extended asm supports input-output or read-write
| operands.  Use the constraint character `+' to indicate such an operand
| and list it with the output operands.
| 
|    When the constraints for the read-write operand (or the operand in
| which only some of the bits are to be changed) allows a register, you
| may, as an alternative, logically split its function into two separate
| operands, one input operand and one write-only output operand.  The
| connection between them is expressed by constraints which say they need
| to be in the same location when the instruction executes.  You can use
| the same C expression for both operands, or different expressions.  For
| example, here we write the (fictitious) `combine' instruction with
| `bar' as its read-only source operand and `foo' as its read-write
| destination:
| 
|      asm ("combine %2,%0" : "=r" (foo) : "0" (foo), "g" (bar));
| 
| The constraint `"0"' for operand 1 says that it must occupy the same
| location as operand 0.  A number in constraint is allowed only in an
| input operand and it must refer to an output operand.
| 
|    Only a number in the constraint can guarantee that one operand will
| be in the same place as another.  The mere fact that `foo' is the value
| of both operands is not enough to guarantee that they will be in the
| same place in the generated assembler code.  The following would not
| work reliably:
| 
|      asm ("combine %2,%0" : "=r" (foo) : "r" (foo), "g" (bar));

Can you explain the need for the change?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] get rid of "+m" constraint in i386 rwsems

[David Howells]

> Can you explain the need for the change?

gcc-3.4 generates warnings about it:

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

The gcc people (or at least one of them) seem to think that these warnings are
correct on a "+m" constraint. See:

	http://marc.theaimsgroup.com/?l=linux-kernel&m=107475162200773&w=2

I understood "+m" to be a shorthand way of specifying "=m" and "m" on the same
bit of memory, but apparently it that's not what it means.

David

Re: [PATCH] get rid of "+m" constraint in i386 rwsems

[Russell King]

On Thu, May 06, 2004 at 01:58:16PM +0100, David Howells wrote:
> > Can you explain the need for the change?
> 
> gcc-3.4 generates warnings about it:
> 
> include/asm/rwsem.h: In function `avc_audit':
> include/asm/rwsem.h:126: warning: read-write constraint does not allow a register
> include/asm/rwsem.h:126: warning: read-write constraint does not allow a register
> 
> The gcc people (or at least one of them) seem to think that these warnings are
> correct on a "+m" constraint. See:
> 
> 	http://marc.theaimsgroup.com/?l=linux-kernel&m=107475162200773&w=2
> 
> I understood "+m" to be a shorthand way of specifying "=m" and "m" on the same
> bit of memory, but apparently it that's not what it means.

After reading Richard's post, I wonder if, in the case of:

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

whether assembly should assume that %0 is the same as %1.  Do they
just happen to be the same thing?  I'm thinking of the case where
there may be two different ways GCC may reference the same memory
location.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] get rid of "+m" constraint in i386 rwsems

[Linus Torvalds]

On Thu, 6 May 2004, David Howells wrote:
> 
> Here's a patch to remove the usage of a "+m" constraint in the i386 optimised
> rwsem implementation.

I think this is wrong, and I thought the gcc people (well, rth seemed to
imply it was a no-brainer) already agreed to support "+m" since:

 - it has long been documented
 - it should be trivial to expand _internally_ in gcc if some 
   implementation thing makes gcc prefer the "=m" / "m" combination.

Richard? 

If it's just a warning in 3.4, and later gcc's are supposed to make it ok 
again, then..

		Linus

Re: [PATCH] get rid of "+m" constraint in i386 rwsems

[Horst von Brand]

Russell King <rmk+lkml@arm.linux.org.uk> said:

[...]

> After reading Richard's post, I wonder if, in the case of:
> 
> 	"=m" (x) : "m" (x)
> 
> whether assembly should assume that %0 is the same as %1.  Do they
> just happen to be the same thing?  I'm thinking of the case where
> there may be two different ways GCC may reference the same memory
> location.

It also might be a "memory location" that isn't really in memory (a local
variable whose value resides in one register up to your asm() fragment, and
from then on in another one or in memory; it might also be useful to load a
value into a register from memory and restore it into memory after sundry
manipulations, and even from a different register, much later).

Today's compilers don't necessarily do things the way a naive understanding
of the source language would say they do. Ever wonder why nobody uses
"register" anymore (compilers are smarter than binding one value to a
register today), and why fiddling with pointers when accessing arrays is
not standard fare (compilers optimize the (bulky, slow) array accesses via
indices out as a matter of course)?
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                     Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria              +56 32 654239
Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513

Re: [PATCH] get rid of "+m" constraint in i386 rwsems

[Richard Henderson]

On Thu, May 06, 2004 at 07:42:51AM -0700, Linus Torvalds wrote:
> If it's just a warning in 3.4, and later gcc's are supposed to make it ok 
> again, then..

Sorry about taking so long to get to this.  It's now fixed for
tree-ssa branch (to be merged to mainline this week) and for
gcc 3.4.1.  Too late for 3.4.0, however...



r~