diff mbox

[ARM] Fix atomic nand operations.

Message ID 1279643588.23062.16.camel@e102325-lin.cambridge.arm.com
State New
Headers show

Commit Message

Ramana Radhakrishnan July 20, 2010, 4:33 p.m. UTC 
Hi ,

Marcus pointed out that the way we implement __sync_nand_and_fetch in
the helper routines is broken today because of the way we test the value
we get.

http://gcc.gnu.org/onlinedocs/gcc-4.4.4/gcc/Atomic-Builtins.html#Atomic-Builtins
and later - 

state that :

Note: GCC 4.4 and later implement __sync_fetch_and_nand builtin as *ptr
= ~(tmp & value) instead of *ptr = ~tmp & value. 

Shouldn't the implementation of this be changed to reflect the semantics
of the documentation in all release branches ?

Ok to commit on trunk and all release branches ?

cheers
Ramana


2010-07-20  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

	* config/arm/linux-atomic.c (FETCH_AND_OP_WORD): Add appropriate
	paranthesis for prefix operations.
	(OP_AND_FETCH_WORD): Likewise.
	(SUBWORD_SYNC_OP): Likewise.

Comments

Ramana Radhakrishnan July 20, 2010, 9:26 p.m. UTC | #1 
On Tue, Jul 20, 2010 at 5:33 PM, Ramana Radhakrishnan
<ramana.radhakrishnan@arm.com> wrote:

> Ok to commit on trunk and all release branches ?

Ofcourse I do mean 4.4 and 4.5 branch :)

Ramana
Joseph Myers July 26, 2010, 10:11 p.m. UTC | #2 
On Tue, 20 Jul 2010, Ramana Radhakrishnan wrote:

> Hi ,
> 
> Marcus pointed out that the way we implement __sync_nand_and_fetch in
> the helper routines is broken today because of the way we test the value
> we get.

What I immediately wondered is why no testcase in the testsuite detected 
this issue.  It looks like the check_effective_target_sync_* do not know 
about sync support on ARM Linux.  Since patches need testcases where 
possible, I think this means you get to fix those functions - or, if that 
does not enable any testcase that fails without the linux-atomic.c change 
and passes with it, to add a new testcase that does show the issue on ARM.
diff mbox

Patch

Index: gcc/config/arm/linux-atomic.c
===================================================================
--- gcc/config/arm/linux-atomic.c	(revision 162305)
+++ gcc/config/arm/linux-atomic.c	(working copy)
@@ -56,7 +56,7 @@  typedef void (__kernel_dmb_t) (void);
 									\
     do {								\
       tmp = *ptr;							\
-      failure = __kernel_cmpxchg (tmp, PFX_OP tmp INF_OP val, ptr);	\
+      failure = __kernel_cmpxchg (tmp, PFX_OP (tmp INF_OP val), ptr);	\
     } while (failure != 0);						\
 									\
     return tmp;								\
@@ -88,8 +88,8 @@  FETCH_AND_OP_WORD (nand, ~, &)
 									\
     do {								\
       oldval = *wordptr;						\
-      newval = ((PFX_OP ((oldval & mask) >> shift)			\
-                 INF_OP (unsigned int) val) << shift) & mask;		\
+      newval = ((PFX_OP (((oldval & mask) >> shift)			\
+			 INF_OP (unsigned int) val) << shift)) & mask;	\
       newval |= oldval & ~mask;						\
       failure = __kernel_cmpxchg (oldval, newval, wordptr);		\
     } while (failure != 0);						\
@@ -119,10 +119,10 @@  SUBWORD_SYNC_OP (nand, ~, &, char, 1, ol
 									\
     do {								\
       tmp = *ptr;							\
-      failure = __kernel_cmpxchg (tmp, PFX_OP tmp INF_OP val, ptr);	\
+      failure = __kernel_cmpxchg (tmp, PFX_OP (tmp INF_OP val), ptr);	\
     } while (failure != 0);						\
 									\
-    return PFX_OP tmp INF_OP val;					\
+    return PFX_OP (tmp INF_OP val);					\
   }
 
 OP_AND_FETCH_WORD (add,   , +)