Re: [PATCH] atomic_dec_if_positive sign extension fix

On Mon, Jan 15, 2007 at 10:09:42AM +1100, Paul Mackerras wrote:

Robert Jennings writes:

quoted

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]

quoted

 	__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