diff mbox

[2/4] ARM 64 bit sync atomic operations [V2]

Message ID 20110726090115.GC6925@davesworkthinkpad
State Superseded
Headers show

Commit Message

Dr. David Alan Gilbert July 26, 2011, 9:01 a.m. UTC
Add ARM 64bit sync helpers for use on older ARMs.  Based on 32bit
    versions but with check for sufficiently new kernel version.
    
        gcc/
    	* config/arm/linux-atomic-64bit.c: New (based on linux-atomic.c)
    	* config/arm/linux-atomic.c: Change comment to point to 64bit version
    	  (SYNC_LOCK_RELEASE): Instantiate 64bit version.
    	* config/arm/t-linux-eabi: Pull in linux-atomic-64bit.c

Comments

Ramana Radhakrishnan Sept. 30, 2011, 4:45 p.m. UTC | #1
On 26 July 2011 10:01, Dr. David Alan Gilbert <david.gilbert@linaro.org> wrote:
>
> +
> +extern unsigned int __write(int fd, const void *buf, unsigned int count);

Why are we using __write instead of write?

A comment elaborating that this file should only be in the static
libgcc and never in the dynamic libgcc would be useful, given that the
constructor is only pulled in only if a 64 bit sync primitive is
referred to.

cheers
Ramana
H.J. Lu Sept. 30, 2011, 5:01 p.m. UTC | #2
On Fri, Sep 30, 2011 at 9:45 AM, Ramana Radhakrishnan
<ramana.radhakrishnan@linaro.org> wrote:
> On 26 July 2011 10:01, Dr. David Alan Gilbert <david.gilbert@linaro.org> wrote:
>>
>> +
>> +extern unsigned int __write(int fd, const void *buf, unsigned int count);
>
> Why are we using __write instead of write?
>
> A comment elaborating that this file should only be in the static
> libgcc and never in the dynamic libgcc would be useful, given that the
> constructor is only pulled in only if a 64 bit sync primitive is
> referred to.
>

You may want to look a look at:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50583

ARM may have the same problem.
Joseph Myers Sept. 30, 2011, 7:54 p.m. UTC | #3
On Fri, 30 Sep 2011, Ramana Radhakrishnan wrote:

> On 26 July 2011 10:01, Dr. David Alan Gilbert <david.gilbert@linaro.org> wrote:
> >
> > +
> > +extern unsigned int __write(int fd, const void *buf, unsigned int count);
> 
> Why are we using __write instead of write?

Because plain write is in the user's namespace in ISO C.  See what I said 
in <http://gcc.gnu.org/ml/gcc-patches/2011-07/msg00084.html> - the 
alternative is hardcoding the syscall number and using the syscall 
directly.
Dr. David Alan Gilbert Oct. 3, 2011, 1:21 p.m. UTC | #4
On 30 September 2011 18:01, H.J. Lu <hjl.tools@gmail.com> wrote:
> You may want to look a look at:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50583
>
> ARM may have the same problem.

OK - although to be honest this patch only stretches the same
structures to 64bit - any major changes in semantics are a separate issue - but
thanks for pointing it out.

Hmm - I think what's produced is correct; however the manual
description is inconsistent:

     These builtins perform the operation suggested by the name, and
     returns the value that had previously been in memory.  That is,

          { tmp = *ptr; *ptr OP= value; return tmp; }

The ARM code (see below) does a single load inside a loop with a guarded
store.  This guarantees that the value returned is the value that
was 'previously been in memory' directly prior to the atomic operation - however
that does mean it doesn't do the pair of accesses implied by the 'tmp
= *ptr; *ptr OP= value'

On ARM the operation for fetch_and_add we get:
(This is pre-my-patch and 32bit, my patch doesn't change the structure
except for the position of that last label):

	mov	r3, r0
	dmb	sy
	.LSYT6:
	ldrex	r0, [r3]
	add	r2, r0, r1
	strex	r0, r2, [r3]
	teq	r0, #0
	bne	.LSYT6
	sub	r0, r2, r1
	dmb	sy

That seems the correct semantics to me - if not what am I missing? Was
the intention of the example
really to cause two loads - if so why?

for sync_and_fetch we get:


	dmb	sy
	.LSYT6:
	ldrex	r0, [r3]
	add	r0, r0, r1
	strex	r2, r0, [r3]
	teq	r2, #0
	bne	.LSYT6
	dmb	sy

i.e. the value returned is always the value that goes into the guarded
store - and is hence
always the value that's stored.

Dave
diff mbox

Patch

diff --git a/gcc/config/arm/linux-atomic-64bit.c b/gcc/config/arm/linux-atomic-64bit.c
new file mode 100644
index 0000000..8b65de8
--- /dev/null
+++ b/gcc/config/arm/linux-atomic-64bit.c
@@ -0,0 +1,162 @@ 
+/* 64bit Linux-specific atomic operations for ARM EABI.
+   Copyright (C) 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
+   Based on linux-atomic.c
+
+   64 bit additions david.gilbert@linaro.org
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+<http://www.gnu.org/licenses/>.  */
+
+/* 64bit helper functions for atomic operations; the compiler will
+   call these when the code is compiled for a CPU without ldrexd/strexd.
+   (If the CPU had those then the compiler inlines the operation).
+
+   These helpers require a kernel helper that's only present on newer
+   kernels; we check for that in an init section and bail out rather
+   unceremoneously.  */
+
+extern unsigned int __write(int fd, const void *buf, unsigned int count);
+extern void abort(void);
+
+/* Kernel helper for compare-and-exchange.  */
+typedef int (__kernel_cmpxchg64_t) (const long long* oldval,
+					const long long* newval,
+					long long *ptr);
+#define __kernel_cmpxchg64 (*(__kernel_cmpxchg64_t *) 0xffff0f60)
+
+/* Kernel helper page version number */
+#define __kernel_helper_version (*(unsigned int *)0xffff0ffc)
+
+/* Check that the kernel has a new enough version at load */
+static void __check_for_sync8_kernelhelper (void)
+{
+  if (__kernel_helper_version < 5)
+    {
+      const char err[] = "A newer kernel is required to run this binary. (__kernel_cmpxchg64 helper)\n";
+      /* At this point we need a way to crash with some information
+	 for the user - I'm not sure I can rely on much else being
+	 available at this point, so do the same as generic-morestack.c
+	 write() and abort(). */
+      __write (2 /* stderr */, err, sizeof(err));
+      abort ();
+    }
+};
+
+static void (*__sync8_kernelhelper_inithook[]) (void) __attribute__ ((used, section (".init_array"))) = {
+  &__check_for_sync8_kernelhelper
+};
+
+#define HIDDEN __attribute__ ((visibility ("hidden")))
+
+#define FETCH_AND_OP_WORD64(OP, PFX_OP, INF_OP)			\
+  long long HIDDEN						\
+  __sync_fetch_and_##OP##_8 (long long *ptr, long long val)	\
+  {								\
+    int failure;						\
+    long long tmp,tmp2;						\
+								\
+    do {							\
+      tmp = *ptr;						\
+      tmp2 = PFX_OP (tmp INF_OP val);				\
+      failure = __kernel_cmpxchg64 (&tmp, &tmp2, ptr);		\
+    } while (failure != 0);					\
+								\
+    return tmp;							\
+  }
+
+FETCH_AND_OP_WORD64 (add,   , +)
+FETCH_AND_OP_WORD64 (sub,   , -)
+FETCH_AND_OP_WORD64 (or,    , |)
+FETCH_AND_OP_WORD64 (and,   , &)
+FETCH_AND_OP_WORD64 (xor,   , ^)
+FETCH_AND_OP_WORD64 (nand, ~, &)
+
+#define NAME_oldval(OP, WIDTH) __sync_fetch_and_##OP##_##WIDTH
+#define NAME_newval(OP, WIDTH) __sync_##OP##_and_fetch_##WIDTH
+
+/* Implement both __sync_<op>_and_fetch and __sync_fetch_and_<op> for
+   subword-sized quantities.  */
+
+#define OP_AND_FETCH_WORD64(OP, PFX_OP, INF_OP)			\
+  long long HIDDEN						\
+  __sync_##OP##_and_fetch_8 (long long *ptr, long long val)	\
+  {								\
+    int failure;						\
+    long long tmp,tmp2;						\
+								\
+    do {							\
+      tmp = *ptr;						\
+      tmp2 = PFX_OP (tmp INF_OP val);				\
+      failure = __kernel_cmpxchg64 (&tmp, &tmp2, ptr);		\
+    } while (failure != 0);					\
+								\
+    return tmp2;						\
+  }
+
+OP_AND_FETCH_WORD64 (add,   , +)
+OP_AND_FETCH_WORD64 (sub,   , -)
+OP_AND_FETCH_WORD64 (or,    , |)
+OP_AND_FETCH_WORD64 (and,   , &)
+OP_AND_FETCH_WORD64 (xor,   , ^)
+OP_AND_FETCH_WORD64 (nand, ~, &)
+
+long long HIDDEN
+__sync_val_compare_and_swap_8 (long long *ptr, long long oldval, long long newval)
+{
+  int failure;
+  long long actual_oldval;
+
+  while (1)
+    {
+      actual_oldval = *ptr;
+
+      if (__builtin_expect (oldval != actual_oldval, 0))
+	return actual_oldval;
+
+      failure = __kernel_cmpxchg64 (&actual_oldval, &newval, ptr);
+
+      if (__builtin_expect (!failure, 1))
+	return oldval;
+    }
+}
+
+typedef unsigned char bool;
+
+bool HIDDEN
+__sync_bool_compare_and_swap_8 (long long *ptr, long long oldval, long long newval)
+{
+  int failure = __kernel_cmpxchg64 (&oldval, &newval, ptr);
+  return (failure == 0);
+}
+
+long long HIDDEN
+__sync_lock_test_and_set_8 (long long *ptr, long long val)
+{
+  int failure;
+  long long oldval;
+
+  do {
+    oldval = *ptr;
+    failure = __kernel_cmpxchg64 (&oldval, &val, ptr);
+  } while (failure != 0);
+
+  return oldval;
+}
diff --git a/gcc/config/arm/linux-atomic.c b/gcc/config/arm/linux-atomic.c
index 57065a6..80f161d 100644
--- a/gcc/config/arm/linux-atomic.c
+++ b/gcc/config/arm/linux-atomic.c
@@ -32,8 +32,8 @@  typedef void (__kernel_dmb_t) (void);
 #define __kernel_dmb (*(__kernel_dmb_t *) 0xffff0fa0)
 
 /* Note: we implement byte, short and int versions of atomic operations using
-   the above kernel helpers, but there is no support for "long long" (64-bit)
-   operations as yet.  */
+   the above kernel helpers; see linux-atomic-64bit.c for "long long" (64-bit)
+   operations.  */
 
 #define HIDDEN __attribute__ ((visibility ("hidden")))
 
@@ -273,6 +273,7 @@  SUBWORD_TEST_AND_SET (unsigned char,  1)
     *ptr = 0;								\
   }
 
+SYNC_LOCK_RELEASE (long long,   8)
 SYNC_LOCK_RELEASE (int,   4)
 SYNC_LOCK_RELEASE (short, 2)
 SYNC_LOCK_RELEASE (char,  1)
diff --git a/gcc/config/arm/t-linux-eabi b/gcc/config/arm/t-linux-eabi
index a328a00..3814cc0 100644
--- a/gcc/config/arm/t-linux-eabi
+++ b/gcc/config/arm/t-linux-eabi
@@ -36,3 +36,4 @@  LIB1ASMFUNCS := $(filter-out _dvmd_tls,$(LIB1ASMFUNCS)) _dvmd_lnx _clear_cache
 EXTRA_MULTILIB_PARTS=crtbegin.o crtend.o crtbeginS.o crtendS.o crtbeginT.o
 
 LIB2FUNCS_STATIC_EXTRA += $(srcdir)/config/arm/linux-atomic.c
+LIB2FUNCS_STATIC_EXTRA += $(srcdir)/config/arm/linux-atomic-64bit.c