This is the mail archive of the
libc-ports@sources.redhat.com
mailing list for the libc-ports project.
Re: [ARM] bits/atomic.h and GCC atomic builtins
On Tuesday, November 16, 2010 4:55:36 pm Joseph S. Myers wrote:
> On Fri, 12 Nov 2010, Ken Werner wrote:
> > Hi,
> >
> > This patch enhances the ARM version of bits/atomic.h to make use of the
> > atomic builtins provided by GCC in case the backend provides a pattern
> > to do this efficiently. Ok to apply?
>
> How exactly did you test this patch?
>
> > +#ifdef GNUC_PREREQ (4, 4)
>
> This is not valid syntax. The compiler should have warned. It looks like
> you mean #if not #ifdef and __GNUC_PREREQ not GNUC_PREREQ.
Yes, this is a typo. I only verified that the GCC emits the correct
instructions for the 4 byte version of the compare-and-swap macro. Later on I
thought it won't hurt to also have the 1,2 and 8 byte versions defined even if
the GCC only emits __sync_val_compare_and_swap_* calls. Therefore I've added
the GNUC_PREREQ (4, 4) macro and made the typo.
> > +/* Use the atomic builtins provided by GCC version 4.4 and later. */
> > +#define __arch_compare_and_exchange_val_8_acq(mem, newval, oldval) \
> > + __sync_val_compare_and_swap ((mem), (oldval), (newval))
> > +#define __arch_compare_and_exchange_val_16_acq(mem, newval, oldval) \
> > + __sync_val_compare_and_swap ((mem), (oldval), (newval))
> > +#define __arch_compare_and_exchange_val_64_acq(mem, newval, oldval) \
> > + __sync_val_compare_and_swap ((mem), (oldval), (newval))
> > +#else
> >
> > #define __arch_compare_and_exchange_val_8_acq(mem, newval, oldval) \
> >
> > ({ __arm_link_error (); oldval; })
> >
> > -
> >
> > #define __arch_compare_and_exchange_val_16_acq(mem, newval, oldval) \
> >
> > ({ __arm_link_error (); oldval; })
> >
> > +#define __arch_compare_and_exchange_val_64_acq(mem, newval, oldval) \
> > + ({ __arm_link_error (); oldval; })
> > +#endif
>
> In any case, the fact that the code worked before shows that these macros
> are not used and so this part of the change serves no purpose. If it were
> useful, conditioning on __GCC_HAVE_SYNC_COMPARE_AND_SWAP_* would probably
> be better than a version conditional, although that would limit it to the
> cases where these operations are inlined.
Ok, sounds reasonable - thanks for your review. Attached is a revised patch
that only touches the __arch_compare_and_exchange_val_32_acq and the
atomic_full_barrier macros. Tested on arm-linux-gnueabi with no regressions.
Regards
Ken
diff --git a/ChangeLog b/ChangeLog
index e42182f..a9bc8fc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2010-11-18 Ken Werner <ken.werner@de.ibm.com>
+
+ * sysdeps/unix/sysv/linux/arm/nptl/bits/atomic.h (atomic_full_barrier,
+ __arch_compare_and_exchange_val_32_acq): Use the atomic builtins
+ provided by GCC if __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 is defined.
+
2010-04-14 Joseph Myers <joseph@codesourcery.com>
* libc-abis: Remove.
diff --git a/sysdeps/unix/sysv/linux/arm/nptl/bits/atomic.h b/sysdeps/unix/sysv/linux/arm/nptl/bits/atomic.h
index b0586ea..979db9f 100644
--- a/sysdeps/unix/sysv/linux/arm/nptl/bits/atomic.h
+++ b/sysdeps/unix/sysv/linux/arm/nptl/bits/atomic.h
@@ -37,7 +37,12 @@ typedef uintmax_t uatomic_max_t;
void __arm_link_error (void);
-#ifdef __thumb2__
+/* Use the atomic builtins provided by GCC in case the backend provides
+ a pattern to do this efficiently. */
+
+#ifdef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
+#define atomic_full_barrier() __sync_synchronize ()
+#elif defined __thumb2__
#define atomic_full_barrier() \
__asm__ __volatile__ \
("movw\tip, #0x0fa0\n\t" \
@@ -64,11 +69,15 @@ void __arm_link_error (void);
#define __arch_compare_and_exchange_val_16_acq(mem, newval, oldval) \
({ __arm_link_error (); oldval; })
+#ifdef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
+#define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval) \
+ __sync_val_compare_and_swap ((mem), (oldval), (newval))
+
/* It doesn't matter what register is used for a_oldval2, but we must
specify one to work around GCC PR rtl-optimization/21223. Otherwise
it may cause a_oldval or a_tmp to be moved to a different register. */
-#ifdef __thumb2__
+#elif defined __thumb2__
/* Thumb-2 has ldrex/strex. However it does not have barrier instructions,
so we still need to use the kernel helper. */
#define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval) \