diff mbox series

[amdgcn] Add support for sub-word sync_compare_and_swap operations

Message ID 08f0684c-0a25-547b-827c-54e9266532d9@codesourcery.com
State New
Headers show
Series [amdgcn] Add support for sub-word sync_compare_and_swap operations | expand

Commit Message

Kwok Cheung Yeung Jan. 8, 2020, 11:07 a.m. UTC 
Hello

This patch adds support for 8- and 16-bit sync_compare_and_swap 
operations on AMD GCN. GCN does not natively support atomic compare and 
swap for quantities smaller than 32-bit words, so the subword compare 
and swap is implemented in terms of 32-bit compare and swap.

The algorithm is similar to that on other architectures (e.g. 
SUBWORD_SYNC_OP in libgcc/config/arm/linux-atomic.c). i.e. It reads from 
memory the word containing the subword, creates a new word by replacing 
the subword with the swap value, then does a 32-bit atomic compare and 
swap with the old and new words. If the operation is unsuccessful due to 
a part of the word that is not the target subword changing values since 
the last read, then it must try again using the updated word.

Okay for trunk?

Kwok


2020-01-08  Kwok Cheung Yeung  <kcy@codesourcery.com>

	libgcc/
	* config/gcn/atomic.c: New.
	* config/gcn/t-amdgcn (LIB2ADD): Add atomic.c.
---
  libgcc/config/gcn/atomic.c | 60 
++++++++++++++++++++++++++++++++++++++++++++++
  libgcc/config/gcn/t-amdgcn |  3 ++-
  2 files changed, 62 insertions(+), 1 deletion(-)
  create mode 100644 libgcc/config/gcn/atomic.c

Comments

Stubbs, Andrew Jan. 8, 2020, 11:42 a.m. UTC | #1 
On 08/01/2020 11:07, Kwok Cheung Yeung wrote:
> +#define __sync_subword_compare_and_swap(type, size)                \

Macro parameters are conventionally upper case.

> +                                        \
> +type                                        \
> +__sync_val_compare_and_swap_##size (type *ptr, type oldval, type 
> newval)    \
> +{                                        \
> +  unsigned int *wordptr                                \
> +    = (unsigned int *)((unsigned long long) ptr & ~3ULL);            \

Please use "intptr_t" rather than "unsigned long long" (which should 
probably have been "unsigned long" anyway).

> +  int shift = ((unsigned long long) ptr & 3ULL) * 8;                \
> +  unsigned int valmask = (1 << (size * 8)) - 1;                    \
> +  unsigned int wordmask = ~(valmask << shift);                    \
> +  unsigned int oldword = *wordptr;                        \
> +  for (;;)                                    \
> +    {                                        \
> +      type prevval = (oldword >> shift) & valmask;                \
> +      if (__builtin_expect (prevval != oldval, 0))                \
> +    return prevval;                                \
> +      unsigned int newword = oldword & wordmask;                \
> +      newword |= ((unsigned int) newval) << shift;                \
> +      unsigned int prevword                            \
> +      = __sync_val_compare_and_swap_4 (wordptr, oldword, 
> newword);        \
> +      if (__builtin_expect (prevword == oldword, 1))                \
> +    return oldval;                                \
> +      oldword = prevword;                            \
> +    }                                        \
> +}                                        \
> +                                        \
> +bool                                        \
> +__sync_bool_compare_and_swap_##size (type *ptr, type oldval, type 
> newval)   \
> +{                                        \
> +  return __sync_val_compare_and_swap_##size(ptr, oldval, newval) == 
> oldval; \

Space before '('.

I presume all the '\' are lined up, but the patch has got mangled in the 
email. Please use an inline attachment.

> +}
> +
> +__sync_subword_compare_and_swap (unsigned char, 1)
> +__sync_subword_compare_and_swap (unsigned short, 2)
> +
> diff --git a/libgcc/config/gcn/t-amdgcn b/libgcc/config/gcn/t-amdgcn
> index adbd866..fe7b5fa 100644
> --- a/libgcc/config/gcn/t-amdgcn
> +++ b/libgcc/config/gcn/t-amdgcn
> @@ -1,4 +1,5 @@
> -LIB2ADD += $(srcdir)/config/gcn/lib2-divmod.c \
> +LIB2ADD += $(srcdir)/config/gcn/atomic.c \
> +       $(srcdir)/config/gcn/lib2-divmod.c \
>          $(srcdir)/config/gcn/lib2-divmod-hi.c \
>          $(srcdir)/config/gcn/unwind-gcn.c
> 

Andrew
Kwok Cheung Yeung Jan. 8, 2020, 6:18 p.m. UTC | #2 
On 08/01/2020 11:42 am, Andrew Stubbs wrote:
> On 08/01/2020 11:07, Kwok Cheung Yeung wrote:
>> +#define __sync_subword_compare_and_swap(type, size)                \
> 
> Macro parameters are conventionally upper case.
> 

Fixed. I upper-cased the macro name as well.

>> +                                        \
>> +type                                        \
>> +__sync_val_compare_and_swap_##size (type *ptr, type oldval, type 
>> newval)    \
>> +{                                        \
>> +  unsigned int *wordptr                                \
>> +    = (unsigned int *)((unsigned long long) ptr & ~3ULL);            \
> 
> Please use "intptr_t" rather than "unsigned long long" (which should 
> probably have been "unsigned long" anyway).
>

I used uintptr_t instead as we are doing unsigned operations (but it 
probably doesn't matter anyway).

>> +__sync_bool_compare_and_swap_##size (type *ptr, type oldval, type 
>> newval)   \
>> +{                                        \
>> +  return __sync_val_compare_and_swap_##size(ptr, oldval, newval) == 
>> oldval; \
> 
> Space before '('.
> 

Fixed.

Is this version okay for trunk?

Thanks

Kwok
From a163377f719e950b0d3820b703029d133ba83637 Mon Sep 17 00:00:00 2001
From: Kwok Cheung Yeung <kcy@codesourcery.com>
Date: Thu, 21 Nov 2019 03:54:46 -0800
Subject: [PATCH] [amdgcn] Add support for sub-word sync_compare_and_swap
 operations

2020-01-08  Kwok Cheung Yeung  <kcy@codesourcery.com>

	libgcc/
	* config/gcn/atomic.c: New.
	* config/gcn/t-amdgcn (LIB2ADD): Add atomic.c.
---
 libgcc/config/gcn/atomic.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++
 libgcc/config/gcn/t-amdgcn |  3 ++-
 2 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 libgcc/config/gcn/atomic.c

diff --git a/libgcc/config/gcn/atomic.c b/libgcc/config/gcn/atomic.c
new file mode 100644
index 0000000..214c9a5
--- /dev/null
+++ b/libgcc/config/gcn/atomic.c
@@ -0,0 +1,60 @@
+/* AMD GCN atomic operations
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   Contributed by Mentor Graphics.
+
+   This file 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.
+
+   This file 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/>.  */
+
+#include <stdbool.h>
+#include <stdint.h>
+
+#define __SYNC_SUBWORD_COMPARE_AND_SWAP(TYPE, SIZE)			     \
+									     \
+TYPE									     \
+__sync_val_compare_and_swap_##SIZE (TYPE *ptr, TYPE oldval, TYPE newval)     \
+{									     \
+  unsigned int *wordptr = (unsigned int *)((uintptr_t) ptr & ~3UL);	     \
+  int shift = ((uintptr_t) ptr & 3UL) * 8;				     \
+  unsigned int valmask = (1 << (SIZE * 8)) - 1;				     \
+  unsigned int wordmask = ~(valmask << shift);				     \
+  unsigned int oldword = *wordptr;					     \
+  for (;;)								     \
+    {									     \
+      TYPE prevval = (oldword >> shift) & valmask;			     \
+      if (__builtin_expect (prevval != oldval, 0))			     \
+	return prevval;							     \
+      unsigned int newword = oldword & wordmask;			     \
+      newword |= ((unsigned int) newval) << shift;			     \
+      unsigned int prevword						     \
+	  = __sync_val_compare_and_swap_4 (wordptr, oldword, newword);	     \
+      if (__builtin_expect (prevword == oldword, 1))			     \
+	return oldval;							     \
+      oldword = prevword;						     \
+    }									     \
+}									     \
+									     \
+bool									     \
+__sync_bool_compare_and_swap_##SIZE (TYPE *ptr, TYPE oldval, TYPE newval)    \
+{									     \
+  return __sync_val_compare_and_swap_##SIZE (ptr, oldval, newval) == oldval; \
+}
+
+__SYNC_SUBWORD_COMPARE_AND_SWAP (unsigned char, 1)
+__SYNC_SUBWORD_COMPARE_AND_SWAP (unsigned short, 2)
+
diff --git a/libgcc/config/gcn/t-amdgcn b/libgcc/config/gcn/t-amdgcn
index adbd866..fe7b5fa 100644
--- a/libgcc/config/gcn/t-amdgcn
+++ b/libgcc/config/gcn/t-amdgcn
@@ -1,4 +1,5 @@
-LIB2ADD += $(srcdir)/config/gcn/lib2-divmod.c \
+LIB2ADD += $(srcdir)/config/gcn/atomic.c \
+	   $(srcdir)/config/gcn/lib2-divmod.c \
 	   $(srcdir)/config/gcn/lib2-divmod-hi.c \
 	   $(srcdir)/config/gcn/unwind-gcn.c
Andrew Stubbs Jan. 9, 2020, 9:42 a.m. UTC | #3 
On 08/01/2020 18:18, Kwok Cheung Yeung wrote:
> Is this version okay for trunk?

OK, thanks.

Andrew
diff mbox series

Patch

diff --git a/libgcc/config/gcn/atomic.c b/libgcc/config/gcn/atomic.c
new file mode 100644
index 0000000..6514dfc
--- /dev/null
+++ b/libgcc/config/gcn/atomic.c
@@ -0,0 +1,60 @@ 
+/* AMD GCN atomic operations
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   Contributed by Mentor Graphics.
+
+   This file 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.
+
+   This file 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/>.  */
+
+#include <stdbool.h>
+
+#define __sync_subword_compare_and_swap(type, size)			    \
+									    \
+type									    \
+__sync_val_compare_and_swap_##size (type *ptr, type oldval, type 
newval)    \
+{									    \
+  unsigned int *wordptr							    \
+	= (unsigned int *)((unsigned long long) ptr & ~3ULL);		    \
+  int shift = ((unsigned long long) ptr & 3ULL) * 8;			    \
+  unsigned int valmask = (1 << (size * 8)) - 1;				    \
+  unsigned int wordmask = ~(valmask << shift);				    \
+  unsigned int oldword = *wordptr;					    \
+  for (;;)								    \
+    {									    \
+      type prevval = (oldword >> shift) & valmask;			    \
+      if (__builtin_expect (prevval != oldval, 0))			    \
+	return prevval;							    \
+      unsigned int newword = oldword & wordmask;			    \
+      newword |= ((unsigned int) newval) << shift;			    \
+      unsigned int prevword						    \
+	  = __sync_val_compare_and_swap_4 (wordptr, oldword, newword);	    \
+      if (__builtin_expect (prevword == oldword, 1))			    \
+	return oldval;							    \
+      oldword = prevword;						    \
+    }									    \
+}									    \
+									    \
+bool									    \
+__sync_bool_compare_and_swap_##size (type *ptr, type oldval, type 
newval)   \
+{									    \
+  return __sync_val_compare_and_swap_##size(ptr, oldval, newval) == 
oldval; \
+}
+
+__sync_subword_compare_and_swap (unsigned char, 1)
+__sync_subword_compare_and_swap (unsigned short, 2)
+
diff --git a/libgcc/config/gcn/t-amdgcn b/libgcc/config/gcn/t-amdgcn
index adbd866..fe7b5fa 100644
--- a/libgcc/config/gcn/t-amdgcn
+++ b/libgcc/config/gcn/t-amdgcn
@@ -1,4 +1,5 @@ 
-LIB2ADD += $(srcdir)/config/gcn/lib2-divmod.c \
+LIB2ADD += $(srcdir)/config/gcn/atomic.c \
+	   $(srcdir)/config/gcn/lib2-divmod.c \
  	   $(srcdir)/config/gcn/lib2-divmod-hi.c \
  	   $(srcdir)/config/gcn/unwind-gcn.c