diff mbox

[committed] Update config/pa/linux-atomic.c and enable sync tests on hppa-linux

Message ID 20101011214203.GA279@hiauly1.hia.nrc.ca
State New
Headers show

Commit Message

John David Anglin Oct. 11, 2010, 9:42 p.m. UTC 
On Wed, 11 Aug 2010, Richard Henderson wrote:

> FWIW, I had a browse through this file and it's a bit off.  E.g.:
> 
> > __sync_val_compare_and_swap_4 (int *ptr, int oldval, int newval)
> > {
> >   int actual_oldval, fail;
> >     
> >   while (1)
> >     {
> >       actual_oldval = *ptr;
> > 
> >       if (oldval != actual_oldval)
> >         return actual_oldval;
> > 
> >       fail = __kernel_cmpxchg (actual_oldval, newval, ptr);
> >   
> >       if (!fail)
> >         return oldval;
> >     }
> > }
> 
> This should not be returning the OLDVAL that the user passed,
> it should be returning the lws_ret that the kernel returned.
> 
> This is because the user of __sync_val_c_a_s wants avoid the
> reload of the value, like so (from libgomp/iter.c):

I adjusted the returns per your suggestion.  Does this look right?

I have had this in my hppa-linux build trees for some time and haven't
seen and regressions.

Thanks for looking over the original patch.

Dave

Comments

Richard Henderson Oct. 11, 2010, 10:31 p.m. UTC | #1 
On 10/11/2010 02:42 PM, John David Anglin wrote:
> I adjusted the returns per your suggestion.  Does this look right?

Looks good.


r~
diff mbox

Patch

Index: config/pa/linux-atomic.c
===================================================================
--- config/pa/linux-atomic.c	(revision 164230)
+++ config/pa/linux-atomic.c	(working copy)
@@ -191,13 +191,13 @@ 
     {
       actual_oldval = *ptr;
 
-      if (oldval != actual_oldval)
+      if (__builtin_expect (oldval != actual_oldval, 0))
 	return actual_oldval;
 
       fail = __kernel_cmpxchg (actual_oldval, newval, ptr);
   
-      if (!fail)
-	return oldval;
+      if (__builtin_expect (!fail, 1))
+	return actual_oldval;
     }
 }
 
@@ -216,8 +216,9 @@ 
       {									\
 	actual_oldval = *wordptr;					\
 									\
-	if (((actual_oldval & mask) >> shift) != (unsigned int) oldval)	\
-          return (actual_oldval & mask) >> shift;			\
+	if (__builtin_expect (((actual_oldval & mask) >> shift)		\
+			      != (unsigned int) oldval, 0))		\
+	  return (actual_oldval & mask) >> shift;			\
 									\
 	actual_newval = (actual_oldval & ~mask)				\
 			| (((unsigned int) newval << shift) & mask);	\
@@ -225,8 +226,8 @@ 
 	fail = __kernel_cmpxchg (actual_oldval, actual_newval,		\
 				 wordptr);				\
 									\
-	if (!fail)							\
-	  return oldval;						\
+	if (__builtin_expect (!fail, 1))				\
+	  return (actual_oldval & mask) >> shift;			\
       }									\
   }