[PATCH] [amdgcn] Add support for sub-word sync_compare_and_swap operations

2020-01-08 Thread Kwok Cheung Yeung 

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  

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

Re: [PATCH] [amdgcn] Add support for sub-word sync_compare_and_swap operations

2020-01-08 Thread Andrew Stubbs 

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

Re: [PATCH] [amdgcn] Add support for sub-word sync_compare_and_swap operations

2020-01-08 Thread Kwok Cheung Yeung 

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 
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  

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 000..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 
+#include 
+
+#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;   \
+}   \
+}

Re: [PATCH] [amdgcn] Add support for sub-word sync_compare_and_swap operations

2020-01-09 Thread Andrew Stubbs 

On 08/01/2020 18:18, Kwok Cheung Yeung wrote:

Is this version okay for trunk?


OK, thanks.

Andrew