Re: [PATCH] atomic_dec_if_positive sign extension fix

[Gabriel Paubert <paubert@iram.es>]

On Mon, Jan 15, 2007 at 10:09:42AM +1100, Paul Mackerras wrote:
> Robert Jennings writes:
> 
> > Please apply for 2.6.20.  If an atomic counter is explicitly set to a
> > negative value the atomic_dec_if_positive function will decrement and
> > store the next smallest value in the atomic counter contrary to it's
> > intended operation.  
> 
> [snip]
> 
> >  	__asm__ __volatile__(
> >  	LWSYNC_ON_SMP
> >  "1:	lwarx	%0,0,%1		# atomic_dec_if_positive\n\
> > +	extsw	%0,%0\n\
> >  	addic.	%0,%0,-1\n\
> >  	blt-	2f\n"
> >  	PPC405_ERR77(0,%1)
> 
> NAK: Good fix for 64-bit, but it will break 32-bit.  I think a better
> fix would be to use a cmpwi after the lwarx, and use addi rather than
> addic..
> 

Indeed, this will also fix the bug that 0x8000000 is considered 
positive since the test was done after the decrement, at the cost
of one additional instrucction in 32 bit mode.

It also avoids clobbering the carry, you don't know whether a future
version of GCC will require explicit clobbers for the carry. For now
they are useless: you can specify "xer" in the clobber list AFAIR but 
no pattern can be split between setters and users of the carry in the
machine description.

	Gabriel