Re: 2.4.x: i386/x86_64 bitops clobberings

[Willy Tarreau]

Hi Jan,

On Mon, Oct 09, 2006 at 08:27:54AM +0200, Jan Kiszka wrote:
> Willy Tarreau wrote:
> > I've done it too, but unfortunately, I discovered that it does not build
> > with gcc-2.95 anymore, while 3.3 is fine :
> > 
> > cyclades.c: In function `cyy_interrupt':
> > /usr/src/git/linux-2.4/include/asm/bitops.h:130: inconsistent operand constraints in an `asm'
> > cyclades.c: In function `cyz_handle_rx':
> > /usr/src/git/linux-2.4/include/asm/bitops.h:130: inconsistent operand constraints in an `asm'
> > 
> > 127        __asm__ __volatile__( LOCK_PREFIX
> > 128                "btsl %2,%1\n\tsbbl %0,%0"
> > 129                :"=r" (oldbit),"+m" (ADDR)
> > 130                :"Ir" (nr) : "memory");
> > 
> > 
> > I don't know what the right solution should be. I'm not fond of #ifdefs,
> > and I'm embarrassed to know that gcc can do all sorts of nasty things due
> > to a wrong clobbering :-/
> > 
> > If you have any idea on the subject, I'm willing to try.
> 
> What about
> 
> #if __GNUC__ < 3
> #define ADDR_DEPS "=m"
> #else
> #define ADDR_DEPS "+m"
> #endif
> 
> 	__asm__ __volatile__( LOCK_PREFIX
> 		"btsl %2,%1\n\tsbbl %0,%0"
> 		:"=r" (oldbit),ADDR_DEPS (ADDR)
> 		:"Ir" (nr) : "memory");
> 
> or something similar? Would keep the number of #ifs low.

I don't like it for two reasons :

  - I don't know how gcc-3.0 to 3.2 behave on such code, so may be this
    would implicitly categorize them in the wrong group

  - it makes the asm code very hard to read. Operand dependencies are
    very sensible tricks which need to be extremely clear.

I have searched the archives for previous occurrences of this recurring
problem, and found another alternative which is said to work on all gcc
versions. So here's the patch for both x86 and x86_64. I checked that it
produces identical code as I obtain with the patch from 2.6. It also
fixes your testcase for gcc-2.95 to 4.1.1.

Could you please give it a try ?

Andi, Linus, I'd be very interested in your comments on this patch, since
you're more used to deal with this problem than me.

Thanks in advance,
Willy