diff mbox

[RFC] powerpc: Change atomic_write_barrier to have release semantics.

Message ID 1413920900.8483.59.camel@triegel.csb
State Committed
Headers

Commit Message

Torvald Riegel Oct. 21, 2014, 7:48 p.m. UTC 
  Based on my scan of existing code, atomic_write_barrier is used as if it
were a C11 memory_order_release fence.  However, powerpc uses "eieio"
currently, which, AFAIK, prohibits reordering of *only* stores.
"lwsync" is used as release fence in other parts of the implementation
(e.g., _rel versions of atomic ops), and it is a valid implementation of
C11 memory_order_release fences.  Therefore, this patch changes
atomic_write_barrier to use "lwsync" when available, or "sync"
otherwise.

I have NOT tested this.  Can somebody who cares about powerpc please
have a look and test?  Thanks!

Comments

Adhemerval Zanella Netto Oct. 31, 2014, 5:47 p.m. UTC | #1 
On 21-10-2014 17:48, Torvald Riegel wrote:
> Based on my scan of existing code, atomic_write_barrier is used as if it
> were a C11 memory_order_release fence.  However, powerpc uses "eieio"
> currently, which, AFAIK, prohibits reordering of *only* stores.
> "lwsync" is used as release fence in other parts of the implementation
> (e.g., _rel versions of atomic ops), and it is a valid implementation of
> C11 memory_order_release fences.  Therefore, this patch changes
> atomic_write_barrier to use "lwsync" when available, or "sync"
> otherwise.
>
> I have NOT tested this.  Can somebody who cares about powerpc please
> have a look and test?  Thanks!

I was about to send a similar patch. This looks ok, thanks.
Torvald Riegel Oct. 31, 2014, 10:50 p.m. UTC | #2 
On Fri, 2014-10-31 at 15:47 -0200, Adhemerval Zanella wrote:
> On 21-10-2014 17:48, Torvald Riegel wrote:
> > Based on my scan of existing code, atomic_write_barrier is used as if it
> > were a C11 memory_order_release fence.  However, powerpc uses "eieio"
> > currently, which, AFAIK, prohibits reordering of *only* stores.
> > "lwsync" is used as release fence in other parts of the implementation
> > (e.g., _rel versions of atomic ops), and it is a valid implementation of
> > C11 memory_order_release fences.  Therefore, this patch changes
> > atomic_write_barrier to use "lwsync" when available, or "sync"
> > otherwise.
> >
> > I have NOT tested this.  Can somebody who cares about powerpc please
> > have a look and test?  Thanks!
> 
> I was about to send a similar patch. This looks ok, thanks.

Committed.
diff mbox

Patch

commit 07653467236190973356ae6a0cfd8cdf329a3d9c
Author: Torvald Riegel <triegel@redhat.com>
Date:   Sat Oct 18 01:01:58 2014 +0200

    powerpc: Change atomic_write_barrier to have release semantics.

diff --git a/sysdeps/powerpc/bits/atomic.h b/sysdeps/powerpc/bits/atomic.h
index b838631..4fc0c5a 100644
--- a/sysdeps/powerpc/bits/atomic.h
+++ b/sysdeps/powerpc/bits/atomic.h
@@ -77,7 +77,6 @@  typedef uintmax_t uatomic_max_t;
 #endif
 
 #define atomic_full_barrier()	__asm ("sync" ::: "memory")
-#define atomic_write_barrier()	__asm ("eieio" ::: "memory")
 
 #define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval)	      \
   ({									      \
diff --git a/sysdeps/powerpc/powerpc32/bits/atomic.h b/sysdeps/powerpc/powerpc32/bits/atomic.h
index 7613bdc..a3dd09c 100644
--- a/sysdeps/powerpc/powerpc32/bits/atomic.h
+++ b/sysdeps/powerpc/powerpc32/bits/atomic.h
@@ -117,6 +117,7 @@ 
 # ifndef UP
 #  define __ARCH_REL_INSTR	"lwsync"
 # endif
+# define atomic_write_barrier()	__asm ("lwsync" ::: "memory")
 #else
 /*
  * Older powerpc32 processors don't support the new "light weight"
@@ -124,6 +125,7 @@ 
  * for all powerpc32 applications.
  */
 # define atomic_read_barrier()	__asm ("sync" ::: "memory")
+# define atomic_write_barrier()	__asm ("sync" ::: "memory")
 #endif
 
 /*
diff --git a/sysdeps/powerpc/powerpc64/bits/atomic.h b/sysdeps/powerpc/powerpc64/bits/atomic.h
index 527fe7c..ed26b72 100644
--- a/sysdeps/powerpc/powerpc64/bits/atomic.h
+++ b/sysdeps/powerpc/powerpc64/bits/atomic.h
@@ -234,6 +234,7 @@ 
 #ifndef UP
 # define __ARCH_REL_INSTR	"lwsync"
 #endif
+#define atomic_write_barrier()	__asm ("lwsync" ::: "memory")
 
 /*
  * Include the rest of the atomic ops macros which are common to both