[PATCH] Add explicit acquire/release semantics to atomic_exchange_and_add.

[Maxim Kuvyrkov <maxim@codesourcery.com>]

On 29/06/2012, at 11:00 AM, Joseph S. Myers wrote:

> On Thu, 28 Jun 2012, Maxim Kuvyrkov wrote:
> 
>> +/* ??? Barrier semantics for atomic_exchange_and_add appear to be
>> +   undefined.  Use full barrier for now, as that's safe.  */
> 
> Please file a bug to clarify these semantics, if not already filed, and 
> reference it in the comment.  (Clarifying the semantics will I suppose 
> involve examining both direct and indirect users of 
> atomic_exchange_and_add to work out what they need and whether it should 
> be split into multiple macros with different barrier semantics.)

This is now http://sourceware.org/bugzilla/show_bug.cgi?id=14350 .

Current generic implementation in include/atomic.h is based on atomic_compare_and_exchange_acq, so it may be that atomic_exchange_and_add implies acquire, but not release semantics.  However, I doubt that the generic implementation was exhaustively tested on multi-processor systems, so we should not blindly depend on this.

As a first step here are patches to add atomic_exchange_and_add_{acq,rel} variants, which then will be used in upcoming optimizations to __libc_lock_lock/__libc_lock_trylock macros and pthread_spin_lock/pthread_spin_trylock implementations.

Tested on mips-linux-gnu.

OK to apply?

--
Maxim Kuvyrkov
CodeSourcery / Mentor Graphics

Add explicit acquire/release semantics to atomic_exchange_and_add.

	2012-07-11  Maxim Kuvyrkov  <maxim@codesourcery.com>

	* include/atomic.h (atomic_exchange_and_add): Split into ...
	(atomic_exchange_and_add_acq, atomic_exchange_and_add_rel): ... these.
	New atomic macros.
---
 include/atomic.h |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/include/atomic.h b/include/atomic.h
index 3ccb46d..bc20772 100644
--- a/include/atomic.h
+++ b/include/atomic.h
@@ -198,8 +198,12 @@
 
 
 /* Add VALUE to *MEM and return the old value of *MEM.  */
-#ifndef atomic_exchange_and_add
-# define atomic_exchange_and_add(mem, value) \
+#ifndef atomic_exchange_and_add_acq
+# ifdef atomic_exchange_and_add
+#  define atomic_exchange_and_add_acq(mem, value) \
+  atomic_exchange_and_add (mem, value)
+# else
+#  define atomic_exchange_and_add_acq(mem, value) \
   ({ __typeof (*(mem)) __atg6_oldval;					      \
      __typeof (mem) __atg6_memp = (mem);				      \
      __typeof (*(mem)) __atg6_value = (value);				      \
@@ -213,8 +217,18 @@
 						   __atg6_oldval), 0));	      \
 									      \
      __atg6_oldval; })
+# endif
 #endif
 
+#ifndef atomic_exchange_and_add_rel
+# define atomic_exchange_and_add_rel(mem, value) \
+  atomic_exchange_and_add_acq(mem, value)
+#endif
+
+#ifndef atomic_exchange_and_add
+# define atomic_exchange_and_add(mem, value) \
+  atomic_exchange_and_add_acq(mem, value)
+#endif
 
 #ifndef catomic_exchange_and_add
 # define catomic_exchange_and_add(mem, value) \
-- 
1.7.4.1

Add explicit acquire/release semantics to atomic_exchange_and_add.

	2012-07-11  Maxim Kuvyrkov  <maxim@codesourcery.com>

	* sysdeps/mips/bit/atomic.h [__GNUC_PREREQ (4, 8)]
	(atomic_exchange_and_add): Split into ...
	(atomic_exchange_and_add_acq, atomic_exchange_and_add_rel): ... these.
	New atomic macros.
	[!__GNUC_PREREQ (4, 8)]
	(atomic_exchange_and_add): Split into ...
	(atomic_exchange_and_add_acq, atomic_exchange_and_add_rel): ... these.
	New atomic macros.
---
 sysdeps/mips/bits/atomic.h |   22 +++++++++++++---------
 1 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/sysdeps/mips/bits/atomic.h b/sysdeps/mips/bits/atomic.h
index b094273..749e166 100644
--- a/sysdeps/mips/bits/atomic.h
+++ b/sysdeps/mips/bits/atomic.h
@@ -193,11 +193,13 @@ typedef uintmax_t uatomic_max_t;
   __atomic_fetch_add (mem, value, model)
 # endif
 
-/* ??? Barrier semantics for atomic_exchange_and_add appear to be
-   undefined.  Use full barrier for now, as that's safe.  */
-# define atomic_exchange_and_add(mem, value)				\
+# define atomic_exchange_and_add_acq(mem, value)			\
   __atomic_val_bysize (__arch_exchange_and_add, int, mem, value,	\
-		       __ATOMIC_ACQ_REL)
+		       __ATOMIC_ACQUIRE)
+
+# define atomic_exchange_and_add_rel(mem, value)			\
+  __atomic_val_bysize (__arch_exchange_and_add, int, mem, value,	\
+		       __ATOMIC_RELEASE)
 #else /* !__GNUC_PREREQ (4, 8) */
 /* This implementation using inline assembly will be removed once glibc
    requires GCC 4.8 or later to build.  */
@@ -434,11 +436,13 @@ typedef uintmax_t uatomic_max_t;
   __prev; })
 # endif
 
-/* ??? Barrier semantics for atomic_exchange_and_add appear to be 
-   undefined.  Use full barrier for now, as that's safe.  */
-# define atomic_exchange_and_add(mem, value) \
-  __atomic_val_bysize (__arch_exchange_and_add, int, mem, value,	      \
-		       MIPS_SYNC_STR, MIPS_SYNC_STR)
+# define atomic_exchange_and_add_acq(mem, value)			\
+  __atomic_val_bysize (__arch_exchange_and_add, int, mem, value,	\
+		       "", MIPS_SYNC_STR)
+
+# define atomic_exchange_and_add_rel(mem, value)			\
+  __atomic_val_bysize (__arch_exchange_and_add, int, mem, value,	\
+		       MIPS_SYNC_STR, "")
 #endif /* __GNUC_PREREQ (4, 8) */
 
 /* TODO: More atomic operations could be implemented efficiently; only the
-- 
1.7.4.1