diff mbox

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

Message ID 20100811022808.GA25554@hiauly1.hia.nrc.ca
State New
Headers show

Commit Message

John David Anglin Aug. 11, 2010, 2:28 a.m. UTC 
The attached patches are essentially identical to that recently applied
to arm.

Tested on hppa-unknown-linux-gnu with no observed regressions.  Committed
to trunk.  Plan to backport to 4.4 and 4.5 after testing.

Dave

Comments

Richard Henderson Aug. 11, 2010, 6:03 p.m. UTC | #1 
On 08/10/2010 07:28 PM, John David Anglin wrote:
> The attached patches are essentially identical to that recently applied
> to arm.
> 
> Tested on hppa-unknown-linux-gnu with no observed regressions.  Committed
> to trunk.  Plan to backport to 4.4 and 4.5 after testing.

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):

>       tmp = __sync_val_compare_and_swap (&ws->next, start, nend);
>       if (__builtin_expect (tmp == start, 1))
>         break;
>       start = tmp;

You might be better writing

static int
__kernel_cmpxchg (int oldval, int newval, int *mem)
{
  do {
    asm(...);
  } while (lws_errno == -EAGAIN);

  /* Other possibilities are EFAULT, ENOSYS, EDEADLOCK.  */
  if (__builtin_expect (lws_errno != 0, 0))
    {
      ABORT_INSTRUCTION;
      __builtin_unreachable ();
    }
    
  return lws_ret;
}

and totally hiding the possibility of kernel failure.

This, incidentally, is exactly the implementation of
__sync_val_c_a_s, with arguments in a different order.


r~
diff mbox

Patch

Index: config/pa/linux-atomic.c
===================================================================
--- config/pa/linux-atomic.c	(revision 162979)
+++ config/pa/linux-atomic.c	(working copy)
@@ -1,5 +1,5 @@ 
 /* Linux-specific atomic operations for PA Linux.
-   Copyright (C) 2008, 2009 Free Software Foundation, Inc.
+   Copyright (C) 2008, 2009, 2010 Free Software Foundation, Inc.
    Based on code contributed by CodeSourcery for ARM EABI Linux.
    Modifications for PA Linux by Helge Deller <deller@gmx.de>
 
@@ -92,7 +92,7 @@ 
 									\
     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;								\
@@ -124,8 +124,8 @@ 
 									\
     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);						\
@@ -133,19 +133,19 @@ 
     return (RETURN & mask) >> shift;					\
   }
 
-SUBWORD_SYNC_OP (add,   , +, short, 2, oldval)
-SUBWORD_SYNC_OP (sub,   , -, short, 2, oldval)
-SUBWORD_SYNC_OP (or,    , |, short, 2, oldval)
-SUBWORD_SYNC_OP (and,   , &, short, 2, oldval)
-SUBWORD_SYNC_OP (xor,   , ^, short, 2, oldval)
-SUBWORD_SYNC_OP (nand, ~, &, short, 2, oldval)
-
-SUBWORD_SYNC_OP (add,   , +, char, 1, oldval)
-SUBWORD_SYNC_OP (sub,   , -, char, 1, oldval)
-SUBWORD_SYNC_OP (or,    , |, char, 1, oldval)
-SUBWORD_SYNC_OP (and,   , &, char, 1, oldval)
-SUBWORD_SYNC_OP (xor,   , ^, char, 1, oldval)
-SUBWORD_SYNC_OP (nand, ~, &, char, 1, oldval)
+SUBWORD_SYNC_OP (add,   , +, unsigned short, 2, oldval)
+SUBWORD_SYNC_OP (sub,   , -, unsigned short, 2, oldval)
+SUBWORD_SYNC_OP (or,    , |, unsigned short, 2, oldval)
+SUBWORD_SYNC_OP (and,   , &, unsigned short, 2, oldval)
+SUBWORD_SYNC_OP (xor,   , ^, unsigned short, 2, oldval)
+SUBWORD_SYNC_OP (nand, ~, &, unsigned short, 2, oldval)
+
+SUBWORD_SYNC_OP (add,   , +, unsigned char, 1, oldval)
+SUBWORD_SYNC_OP (sub,   , -, unsigned char, 1, oldval)
+SUBWORD_SYNC_OP (or,    , |, unsigned char, 1, oldval)
+SUBWORD_SYNC_OP (and,   , &, unsigned char, 1, oldval)
+SUBWORD_SYNC_OP (xor,   , ^, unsigned char, 1, oldval)
+SUBWORD_SYNC_OP (nand, ~, &, unsigned char, 1, oldval)
 
 #define OP_AND_FETCH_WORD(OP, PFX_OP, INF_OP)				\
   int HIDDEN								\
@@ -155,10 +155,10 @@ 
 									\
     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,   , +)
@@ -168,19 +168,19 @@ 
 OP_AND_FETCH_WORD (xor,   , ^)
 OP_AND_FETCH_WORD (nand, ~, &)
 
-SUBWORD_SYNC_OP (add,   , +, short, 2, newval)
-SUBWORD_SYNC_OP (sub,   , -, short, 2, newval)
-SUBWORD_SYNC_OP (or,    , |, short, 2, newval)
-SUBWORD_SYNC_OP (and,   , &, short, 2, newval)
-SUBWORD_SYNC_OP (xor,   , ^, short, 2, newval)
-SUBWORD_SYNC_OP (nand, ~, &, short, 2, newval)
-
-SUBWORD_SYNC_OP (add,   , +, char, 1, newval)
-SUBWORD_SYNC_OP (sub,   , -, char, 1, newval)
-SUBWORD_SYNC_OP (or,    , |, char, 1, newval)
-SUBWORD_SYNC_OP (and,   , &, char, 1, newval)
-SUBWORD_SYNC_OP (xor,   , ^, char, 1, newval)
-SUBWORD_SYNC_OP (nand, ~, &, char, 1, newval)
+SUBWORD_SYNC_OP (add,   , +, unsigned short, 2, newval)
+SUBWORD_SYNC_OP (sub,   , -, unsigned short, 2, newval)
+SUBWORD_SYNC_OP (or,    , |, unsigned short, 2, newval)
+SUBWORD_SYNC_OP (and,   , &, unsigned short, 2, newval)
+SUBWORD_SYNC_OP (xor,   , ^, unsigned short, 2, newval)
+SUBWORD_SYNC_OP (nand, ~, &, unsigned short, 2, newval)
+
+SUBWORD_SYNC_OP (add,   , +, unsigned char, 1, newval)
+SUBWORD_SYNC_OP (sub,   , -, unsigned char, 1, newval)
+SUBWORD_SYNC_OP (or,    , |, unsigned char, 1, newval)
+SUBWORD_SYNC_OP (and,   , &, unsigned char, 1, newval)
+SUBWORD_SYNC_OP (xor,   , ^, unsigned char, 1, newval)
+SUBWORD_SYNC_OP (nand, ~, &, unsigned char, 1, newval)
 
 int HIDDEN
 __sync_val_compare_and_swap_4 (int *ptr, int oldval, int newval)
@@ -230,8 +230,8 @@ 
       }									\
   }
 
-SUBWORD_VAL_CAS (short, 2)
-SUBWORD_VAL_CAS (char,  1)
+SUBWORD_VAL_CAS (unsigned short, 2)
+SUBWORD_VAL_CAS (unsigned char,  1)
 
 typedef unsigned char bool;
 
@@ -252,8 +252,8 @@ 
     return (oldval == actual_oldval);					\
   }
 
-SUBWORD_BOOL_CAS (short, 2)
-SUBWORD_BOOL_CAS (char,  1)
+SUBWORD_BOOL_CAS (unsigned short, 2)
+SUBWORD_BOOL_CAS (unsigned char,  1)
 
 int HIDDEN
 __sync_lock_test_and_set_4 (int *ptr, int val)
@@ -289,8 +289,8 @@ 
     return (oldval & mask) >> shift;					\
   }
 
-SUBWORD_TEST_AND_SET (short, 2)
-SUBWORD_TEST_AND_SET (char,  1)
+SUBWORD_TEST_AND_SET (unsigned short, 2)
+SUBWORD_TEST_AND_SET (unsigned char,  1)
 
 #define SYNC_LOCK_RELEASE(TYPE, WIDTH)					\
   void HIDDEN								\