public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [PATCH, HPPA] Atomic builtins using kernel helpers for Linux
@ 2008-07-13 20:34 John David Anglin
  2008-07-13 20:39 ` Helge Deller
  0 siblings, 1 reply; 11+ messages in thread
From: John David Anglin @ 2008-07-13 20:34 UTC (permalink / raw)
  To: gcc-patches; +Cc: deller, carlos

> Below is the equivalent patch for HPPA.
> As mentioned before, there are not much differences between this version and
> the ARM version.

I like the idea of this patch but I believe that this needs careful
thought before the interface is cast in stone.

+/* Determine kernel LWS function call (0=32bit, 1=64bit userspace)  */
+#define LWS_CAS (sizeof(unsigned long) == 4 ? 0 : 1)
+
+/* Kernel helper for compare-and-exchange.  */
+#define __kernel_cmpxchg( oldval, newval, mem )                                \
+  ({                                                                   \
+    register long lws_ret   asm("r28");                                        \
+    register long lws_errno asm("r21");                                        \
+    register unsigned long lws_mem asm("r26") = (unsigned long) (mem); \
+    register long lws_old asm("r25") = (oldval);                       \
+    register long lws_new asm("r24") = (newval);                       \
+    asm volatile(      "ble    0xb0(%%sr2, %%r0)       \n\t"           \
+                       "ldi    %5, %%r20               \n\t"           \
+       : "=r" (lws_ret), "=r" (lws_errno), "=r" (lws_mem),             \
+         "=r" (lws_old), "=r" (lws_new)                                \
+       : "i" (LWS_CAS), "2" (lws_mem), "3" (lws_old), "4" (lws_new)    \
+       : "r1", "r20", "r22", "r23", "r31", "memory"                    \
+    );                                                                         \
+    lws_errno;                                                         \
+   })

From a style standpoint, GCC macro defines and their arguments are usually
in upper case.  There is no space before and after the arguments.

You are using the 32-bit and 64-bit cmpxchg's in the 32 and 64-bit
runtimes, respectively.  However, the implementation that's currently
in the kernel for lws_compare_and_swap64 operates on a 32-bit object.
Thus, the type for oldval and newval should be int.

Possibly, the kernel implementation should be modified to add byte,
half and dword operations.  This would avoid some of the shift and
mask operations in the subsequent defines.  There's currently no
64-bit runtime, so changing lws_compare_and_swap64 shouldn't be a
problem.

I would like to see kernel support for lws_compare_and_swap8,
lws_compare_and_swap16, lws_compare_and_swap32 on 32-bit kernels.
I would also like to see lws_compare_and_swap64 changed to do
a 64-bit swap.  Hopefully, this could be done without running
out of space on the gateway page.

+/* Kernel helper for memory barrier.  */
+#define __kernel_dmb() asm volatile ( "" : : : "memory" );

Comment for above?

+/* 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.  */

This comment assumes 32-bit runtime.  "long long" and "long" are
both 64 bits in the 64-bit runtime.  This swap could be done easily
with a 64-bit kernel.

+#if __BYTE_ORDER == __LITTLE_ENDIAN
+#define INVERT_MASK_1 0
+#define INVERT_MASK_2 0
+#elif __BYTE_ORDER == __BIG_ENDIAN
+#define INVERT_MASK_1 24
+#define INVERT_MASK_2 16
+#else
+#error "Endianess missing"
+#endif

As far as I know, GCC supports no working little endian implementations
on PA-RISC, so the little endian defines are just additional clutter.

+#define MASK_1 0xffu
+#define MASK_2 0xffffu

With the above kernel support, I am hoping the mask defines will
go away.

+#define SUBWORD_SYNC_OP(OP, PFX_OP, INF_OP, TYPE, WIDTH, RETURN)       \
+  TYPE HIDDEN                                                          \
+  NAME##_##RETURN (OP, WIDTH) (TYPE *ptr, TYPE val)                    \
+  {                                                                    \
+    int *wordptr = (int *) ((unsigned int) ptr & ~3);                  \

The cast is wrong for 64-bit runtime.  Should be unsigned long.

+    unsigned int mask, shift, oldval, newval;                          \
+    int failure;                                                       \
+                                                                       \
+    shift = (((unsigned int) ptr & 3) << 3) ^ INVERT_MASK_##WIDTH;     \

Ditto.

+    int *wordptr = (int *)((unsigned int) ptr & ~3), fail;             \

Ditto.

+    shift = (((unsigned int) ptr & 3) << 3) ^ INVERT_MASK_##WIDTH;     \

Ditto.

+    int *wordptr = (int *) ((unsigned int) ptr & ~3);                  \

Ditto.

+    shift = (((unsigned int) ptr & 3) << 3) ^ INVERT_MASK_##WIDTH;     \

Ditto.

I don't like very much the fact that the implementations loop forever
when EFAULT or ENOSYS is returned by the kernel.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

^ permalink raw reply	[flat|nested] 11+ messages in thread
[parent not found: <487BBC76.9000204@gmx.de>]
* [PATCH, ARM] Atomic builtins using kernel helpers for Linux/EABI
@ 2008-07-01 17:16 Julian Brown
  2008-07-09 22:36 ` Helge Deller
  0 siblings, 1 reply; 11+ messages in thread
From: Julian Brown @ 2008-07-01 17:16 UTC (permalink / raw)
  To: gcc-patches; +Cc: paul

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]

Hi,

This patch implements the atomic builtins described at:

  http://gcc.gnu.org/onlinedocs/gcc-4.3.0/gcc/Atomic-Builtins.html

for ARM EABI Linux. This implementation uses the kernel helpers
__kernel_cmpxchg and __kernel_dmb, and so should work on any
architecture which supports those. (More-efficient versions are possible
using ldrex/strex on architectures >=v6, but those are not written yet.)

Atomic operations are provided for data sizes of 1, 2 and 4 bytes (but
not 8 bytes). The implementation uses actual functions
(__sync_fetch_and_add_2, etc.) rather than expanding code inline.

Tested with cross to arm-none-linux-gnueabi, and with some additional
hand-written tests which hopefully exercised the atomicity of the
operations sufficiently.

OK for mainline?

Julian

ChangeLog

    gcc/
    * config/arm/t-linux-eabi (LIB2FUNCS_STATIC_EXTRA): Add
    config/arm/linux-atomic.c.
    * config/arm/linux-atomic.c: New.

[-- Attachment #2: arm-atomics-fsf-1.diff --]
[-- Type: text/x-patch, Size: 9494 bytes --]

Index: gcc/config/arm/linux-atomic.c
===================================================================
--- gcc/config/arm/linux-atomic.c	(revision 0)
+++ gcc/config/arm/linux-atomic.c	(revision 0)
@@ -0,0 +1,280 @@
+/* Linux-specific atomic operations for ARM EABI.
+   Copyright (C) 2008 Free Software Foundation, Inc.
+   Contributed by CodeSourcery.
+
+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 2, or (at your option) any later
+version.
+
+In addition to the permissions in the GNU General Public License, the
+Free Software Foundation gives you unlimited permission to link the
+compiled version of this file into combinations with other programs,
+and to distribute those combinations without any restriction coming
+from the use of this file.  (The General Public License restrictions
+do apply in other respects; for example, they cover modification of
+the file, and distribution when not linked into a combine
+executable.)
+
+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.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING.  If not, write to the Free
+Software Foundation, 51 Franklin Street, Fifth Floor, Boston, MA
+02110-1301, USA.  */
+
+/* Kernel helper for compare-and-exchange.  */
+typedef int (__kernel_cmpxchg_t) (int oldval, int newval, int *ptr);
+#define __kernel_cmpxchg (*(__kernel_cmpxchg_t *) 0xffff0fc0)
+
+/* Kernel helper for memory barrier.  */
+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.  */
+
+#define HIDDEN __attribute__ ((visibility ("hidden")))
+
+#ifdef __ARMEL__
+#define INVERT_MASK_1 0
+#define INVERT_MASK_2 0
+#else
+#define INVERT_MASK_1 24
+#define INVERT_MASK_2 16
+#endif
+
+#define MASK_1 0xffu
+#define MASK_2 0xffffu
+
+#define FETCH_AND_OP_WORD(OP, PFX_OP, INF_OP)				\
+  int HIDDEN								\
+  __sync_fetch_and_##OP##_4 (int *ptr, int val)				\
+  {									\
+    int failure, tmp;							\
+									\
+    do {								\
+      tmp = *ptr;							\
+      failure = __kernel_cmpxchg (tmp, PFX_OP tmp INF_OP val, ptr);	\
+    } while (failure != 0);						\
+									\
+    return tmp;								\
+  }
+
+FETCH_AND_OP_WORD (add,   , +)
+FETCH_AND_OP_WORD (sub,   , -)
+FETCH_AND_OP_WORD (or,    , |)
+FETCH_AND_OP_WORD (and,   , &)
+FETCH_AND_OP_WORD (xor,   , ^)
+FETCH_AND_OP_WORD (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 SUBWORD_SYNC_OP(OP, PFX_OP, INF_OP, TYPE, WIDTH, RETURN)	\
+  TYPE HIDDEN								\
+  NAME##_##RETURN (OP, WIDTH) (TYPE *ptr, TYPE val)			\
+  {									\
+    int *wordptr = (int *) ((unsigned int) ptr & ~3);			\
+    unsigned int mask, shift, oldval, newval;				\
+    int failure;							\
+									\
+    shift = (((unsigned int) ptr & 3) << 3) ^ INVERT_MASK_##WIDTH;	\
+    mask = MASK_##WIDTH << shift;					\
+									\
+    do {								\
+      oldval = *wordptr;						\
+      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);						\
+									\
+    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)
+
+#define OP_AND_FETCH_WORD(OP, PFX_OP, INF_OP)				\
+  int HIDDEN								\
+  __sync_##OP##_and_fetch_4 (int *ptr, int val)				\
+  {									\
+    int tmp, failure;							\
+									\
+    do {								\
+      tmp = *ptr;							\
+      failure = __kernel_cmpxchg (tmp, PFX_OP tmp INF_OP val, ptr);	\
+    } while (failure != 0);						\
+									\
+    return PFX_OP tmp INF_OP val;					\
+  }
+
+OP_AND_FETCH_WORD (add,   , +)
+OP_AND_FETCH_WORD (sub,   , -)
+OP_AND_FETCH_WORD (or,    , |)
+OP_AND_FETCH_WORD (and,   , &)
+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)
+
+int HIDDEN
+__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;
+    }
+}
+
+#define SUBWORD_VAL_CAS(TYPE, WIDTH)					\
+  TYPE HIDDEN								\
+  __sync_val_compare_and_swap_##WIDTH (TYPE *ptr, TYPE oldval,		\
+				       TYPE newval)			\
+  {									\
+    int *wordptr = (int *)((unsigned int) ptr & ~3), fail;		\
+    unsigned int mask, shift, actual_oldval, actual_newval;		\
+									\
+    shift = (((unsigned int) ptr & 3) << 3) ^ INVERT_MASK_##WIDTH;	\
+    mask = MASK_##WIDTH << shift;					\
+									\
+    while (1)								\
+      {									\
+	actual_oldval = *wordptr;					\
+									\
+	if (((actual_oldval & mask) >> shift) != (unsigned int) oldval)	\
+          return (actual_oldval & mask) >> shift;			\
+									\
+	actual_newval = (actual_oldval & ~mask)				\
+			| (((unsigned int) newval << shift) & mask);	\
+									\
+	fail = __kernel_cmpxchg (actual_oldval, actual_newval,		\
+				 wordptr);				\
+									\
+	if (!fail)							\
+          return oldval;						\
+      }									\
+  }
+
+SUBWORD_VAL_CAS (short, 2)
+SUBWORD_VAL_CAS (char,  1)
+
+typedef unsigned char bool;
+
+bool HIDDEN
+__sync_bool_compare_and_swap_4 (int *ptr, int oldval, int newval)
+{
+  int failure = __kernel_cmpxchg (oldval, newval, ptr);
+  return (failure == 0);
+}
+
+#define SUBWORD_BOOL_CAS(TYPE, WIDTH)					\
+  bool HIDDEN								\
+  __sync_bool_compare_and_swap_##WIDTH (TYPE *ptr, TYPE oldval,		\
+					TYPE newval)			\
+  {									\
+    TYPE actual_oldval							\
+      = __sync_val_compare_and_swap_##WIDTH (ptr, oldval, newval);	\
+    return (oldval == actual_oldval);					\
+  }
+
+SUBWORD_BOOL_CAS (short, 2)
+SUBWORD_BOOL_CAS (char,  1)
+
+void HIDDEN
+__sync_synchronize (void)
+{
+  __kernel_dmb ();
+}
+
+int HIDDEN
+__sync_lock_test_and_set_4 (int *ptr, int val)
+{
+  int failure, oldval;
+
+  do {
+    oldval = *ptr;
+    failure = __kernel_cmpxchg (oldval, val, ptr);
+  } while (failure != 0);
+
+  return oldval;
+}
+
+#define SUBWORD_TEST_AND_SET(TYPE, WIDTH)				\
+  TYPE HIDDEN								\
+  __sync_lock_test_and_set_##WIDTH (TYPE *ptr, TYPE val)		\
+  {									\
+    int failure;							\
+    unsigned int oldval, newval, shift, mask;				\
+    int *wordptr = (int *) ((unsigned int) ptr & ~3);			\
+									\
+    shift = (((unsigned int) ptr & 3) << 3) ^ INVERT_MASK_##WIDTH;	\
+    mask = MASK_##WIDTH << shift;					\
+									\
+    do {								\
+      oldval = *wordptr;						\
+      newval = (oldval & ~mask)						\
+	       | (((unsigned int) val << shift) & mask);		\
+      failure = __kernel_cmpxchg (oldval, newval, wordptr);		\
+    } while (failure != 0);						\
+									\
+    return (oldval & mask) >> shift;					\
+  }
+
+SUBWORD_TEST_AND_SET (short, 2)
+SUBWORD_TEST_AND_SET (char,  1)
+
+#define SYNC_LOCK_RELEASE(TYPE, WIDTH)					\
+  void HIDDEN								\
+  __sync_lock_release_##WIDTH (TYPE *ptr)				\
+  {									\
+    *ptr = 0;								\
+    __kernel_dmb ();							\
+  }
+
+SYNC_LOCK_RELEASE (int,   4)
+SYNC_LOCK_RELEASE (short, 2)
+SYNC_LOCK_RELEASE (char,  1)
Index: gcc/config/arm/t-linux-eabi
===================================================================
--- gcc/config/arm/t-linux-eabi	(revision 136167)
+++ gcc/config/arm/t-linux-eabi	(working copy)
@@ -12,3 +12,5 @@ LIB1ASMFUNCS := $(filter-out _dvmd_tls,$
 # Multilib the standard Linux files.  Don't include crti.o or crtn.o,
 # which are provided by glibc.
 EXTRA_MULTILIB_PARTS=crtbegin.o crtend.o crtbeginS.o crtendS.o crtbeginT.o
+
+LIB2FUNCS_STATIC_EXTRA += $(srcdir)/config/arm/linux-atomic.c

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2008-10-30  9:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-07-13 20:34 [PATCH, HPPA] Atomic builtins using kernel helpers for Linux John David Anglin
2008-07-13 20:39 ` Helge Deller
2008-07-14  0:02   ` Helge Deller
2008-07-15  0:34     ` [PATCH, HPPA] Atomic builtins using kernel helpers for Linux (try #4) Helge Deller
2008-10-29 14:00       ` PING " Andrew Haley
2008-10-29 23:50         ` Helge Deller
2008-10-30 12:13           ` Andrew Haley
2008-07-14  1:46   ` [PATCH, HPPA] Atomic builtins using kernel helpers for Linux John David Anglin
2008-07-14 13:37     ` Carlos O'Donell
     [not found] <487BBC76.9000204@gmx.de>
2008-09-07 17:20 ` John David Anglin
  -- strict thread matches above, loose matches on Subject: below --
2008-07-01 17:16 [PATCH, ARM] Atomic builtins using kernel helpers for Linux/EABI Julian Brown
2008-07-09 22:36 ` Helge Deller
2008-07-10 14:10   ` Andrew Haley
2008-07-10 15:37     ` Carlos O'Donell
2008-07-10 15:54       ` Daniel Jacobowitz
2008-07-10 19:57         ` Mark Mitchell
2008-07-13 16:26           ` [PATCH, HPPA] Atomic builtins using kernel helpers for Linux Helge Deller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).