get rid of "+m" constraint in i386 rwsems

Hi Linus, Andrew,

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

David

@@ -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;
 }

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

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

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

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

Russell King <[email protected]> said:

[...]

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

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~