public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v3 0/9] [RISC-V] Atomics improvements
@ 2022-05-27  6:07 Christoph Muellner
  2022-05-27  6:07 ` [PATCH v3 1/9] RISC-V: Simplify memory model code [PR 100265] Christoph Muellner
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Christoph Muellner @ 2022-05-27  6:07 UTC (permalink / raw)
  To: gcc-patches, Kito Cheng, Jim Wilson, Andrew Waterman,
	Philipp Tomsich, Christoph Muellner, Aaron Durbin,
	Patrick O'Neill, Vineet Gupta

This series provides a cleanup of the current atomics implementation
of RISC-V (PR100265: Use proper fences for atomic load/store).

The first patch could be squashed into the following patches,
but I found it easier to understand the chances with it in place.

The series has been tested as follows:
* Building and testing a multilib RV32/64 toolchain
  (bootstrapped with riscv-gnu-toolchain repo)
* Manual review of generated sequences for GCC's atomic builtins API

This series was developed more than a year ago, but got never merged.

v1 can be found here:
https://gcc.gnu.org/pipermail/gcc-patches/2021-April/568684.html

v2 can be found here:
https://gcc.gnu.org/pipermail/gcc-patches/2021-May/569691.html

Jim expressed concerns about patch 9/10 (which was inspired by the
AArch64 implementation), that it won't emit the expected CAS sequence
under register pressure. Therefore, I've dropped the patch from the
series in v3.

Changes for v3:
* Rebase/retest on master
* Drop patch 9/10 ("Provide programmatic implementation of CAS")

Changes for v2:
* Guard LL/SC sequence by compiler barriers ("blockage")
  (suggested by Andrew Waterman)
* Changed commit message for AMOSWAP->STORE change
  (suggested by Andrew Waterman)
* Extracted cbranch4 patch from patchset (suggested by Kito Cheng)
* Introduce predicate riscv_sync_memory_operand (suggested by Jim Wilson)
* Fix small code style issue

Christoph Muellner (9):
  RISC-V: Simplify memory model code [PR 100265]
  RISC-V: Emit proper memory ordering suffixes for AMOs [PR 100265]
  RISC-V: Eliminate %F specifier from riscv_print_operand() [PR 100265]
  RISC-V: Use STORE instead of AMOSWAP for atomic stores [PR 100265]
  RISC-V: Emit fences according to chosen memory model [PR 100265]
  RISC-V: Implement atomic_{load,store} [PR 100265]
  RISC-V: Model INSNs for LR and SC [PR 100266]
  RISC-V: Add s.ext-consuming INSNs for LR and SC [PR 100266]
  RISC-V: Introduce predicate "riscv_sync_memory_operand" [PR 100266]

 gcc/config/riscv/riscv.cc |  61 +++----------
 gcc/config/riscv/sync.md  | 183 ++++++++++++++++++++++++++++++--------
 2 files changed, 159 insertions(+), 85 deletions(-)

-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/9] RISC-V: Simplify memory model code [PR 100265]
  2022-05-27  6:07 [PATCH v3 0/9] [RISC-V] Atomics improvements Christoph Muellner
@ 2022-05-27  6:07 ` Christoph Muellner
  2022-05-27  6:07 ` [PATCH v3 2/9] RISC-V: Emit proper memory ordering suffixes for AMOs " Christoph Muellner
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Muellner @ 2022-05-27  6:07 UTC (permalink / raw)
  To: gcc-patches, Kito Cheng, Jim Wilson, Andrew Waterman,
	Philipp Tomsich, Christoph Muellner, Aaron Durbin,
	Patrick O'Neill, Vineet Gupta

We don't have any special treatment of MEMMODEL_SYNC_* values,
so let's hide them behind the memmodel_base() function.

    gcc/
	PR 100265
        * config/riscv/riscv.c (riscv_memmodel_needs_amo_acquire):
	  Ignore MEMMODEL_SYNC_* values.
        * config/riscv/riscv.c (riscv_memmodel_needs_release_fence):
	  Likewise.
        * config/riscv/riscv.c (riscv_print_operand): Eliminate
	  MEMMODEL_SYNC_* values by calling memmodel_base().
---
 gcc/config/riscv/riscv.cc | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index f83dc796d88..1a130f1fe3b 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -3578,20 +3578,17 @@ riscv_print_operand_reloc (FILE *file, rtx op, bool hi_reloc)
    acquire portion of memory model MODEL.  */
 
 static bool
-riscv_memmodel_needs_amo_acquire (enum memmodel model)
+riscv_memmodel_needs_amo_acquire (const enum memmodel model)
 {
   switch (model)
     {
       case MEMMODEL_ACQ_REL:
       case MEMMODEL_SEQ_CST:
-      case MEMMODEL_SYNC_SEQ_CST:
       case MEMMODEL_ACQUIRE:
       case MEMMODEL_CONSUME:
-      case MEMMODEL_SYNC_ACQUIRE:
 	return true;
 
       case MEMMODEL_RELEASE:
-      case MEMMODEL_SYNC_RELEASE:
       case MEMMODEL_RELAXED:
 	return false;
 
@@ -3604,20 +3601,17 @@ riscv_memmodel_needs_amo_acquire (enum memmodel model)
    implement the release portion of memory model MODEL.  */
 
 static bool
-riscv_memmodel_needs_release_fence (enum memmodel model)
+riscv_memmodel_needs_release_fence (const enum memmodel model)
 {
   switch (model)
     {
       case MEMMODEL_ACQ_REL:
       case MEMMODEL_SEQ_CST:
-      case MEMMODEL_SYNC_SEQ_CST:
       case MEMMODEL_RELEASE:
-      case MEMMODEL_SYNC_RELEASE:
 	return true;
 
       case MEMMODEL_ACQUIRE:
       case MEMMODEL_CONSUME:
-      case MEMMODEL_SYNC_ACQUIRE:
       case MEMMODEL_RELAXED:
 	return false;
 
@@ -3644,6 +3638,7 @@ riscv_print_operand (FILE *file, rtx op, int letter)
 {
   machine_mode mode = GET_MODE (op);
   enum rtx_code code = GET_CODE (op);
+  const enum memmodel model = memmodel_base (INTVAL (op));
 
   switch (letter)
     {
@@ -3663,12 +3658,12 @@ riscv_print_operand (FILE *file, rtx op, int letter)
       break;
 
     case 'A':
-      if (riscv_memmodel_needs_amo_acquire ((enum memmodel) INTVAL (op)))
+      if (riscv_memmodel_needs_amo_acquire (model))
 	fputs (".aq", file);
       break;
 
     case 'F':
-      if (riscv_memmodel_needs_release_fence ((enum memmodel) INTVAL (op)))
+      if (riscv_memmodel_needs_release_fence (model))
 	fputs ("fence iorw,ow; ", file);
       break;
 
-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 2/9] RISC-V: Emit proper memory ordering suffixes for AMOs [PR 100265]
  2022-05-27  6:07 [PATCH v3 0/9] [RISC-V] Atomics improvements Christoph Muellner
  2022-05-27  6:07 ` [PATCH v3 1/9] RISC-V: Simplify memory model code [PR 100265] Christoph Muellner
@ 2022-05-27  6:07 ` Christoph Muellner
  2022-05-27  6:07 ` [PATCH v3 3/9] RISC-V: Eliminate %F specifier from riscv_print_operand() " Christoph Muellner
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Muellner @ 2022-05-27  6:07 UTC (permalink / raw)
  To: gcc-patches, Kito Cheng, Jim Wilson, Andrew Waterman,
	Philipp Tomsich, Christoph Muellner, Aaron Durbin,
	Patrick O'Neill, Vineet Gupta

The ratified A extension supports '.aq', '.rl' and '.aqrl' as
memory ordering suffixes. Let's emit them in case we get a '%A'
conversion specifier for riscv_print_operand().

As '%A' was already used for a similar, but restricted, purpose
(only '.aq' was emitted so far), this does not require any other
changes.

    gcc/
        PR 100265
        * config/riscv/riscv.c (riscv_memmodel_needs_amo_acquire):
          Remove function.
        * config/riscv/riscv.c (riscv_print_amo_memory_ordering_suffix):
          Add function to emit AMO memory ordering suffixes.
        * config/riscv/riscv.c (riscv_print_operand): Call
          riscv_print_amo_memory_ordering_suffix() instead of
          riscv_memmodel_needs_amo_acquire().
---
 gcc/config/riscv/riscv.cc | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 1a130f1fe3b..983a567c69c 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -3574,24 +3574,26 @@ riscv_print_operand_reloc (FILE *file, rtx op, bool hi_reloc)
   fputc (')', file);
 }
 
-/* Return true if the .AQ suffix should be added to an AMO to implement the
-   acquire portion of memory model MODEL.  */
+/* Print the memory ordering suffix for AMOs.  */
 
-static bool
-riscv_memmodel_needs_amo_acquire (const enum memmodel model)
+static void
+riscv_print_amo_memory_ordering_suffix (FILE *file, const enum memmodel model)
 {
   switch (model)
     {
-      case MEMMODEL_ACQ_REL:
-      case MEMMODEL_SEQ_CST:
-      case MEMMODEL_ACQUIRE:
+      case MEMMODEL_RELAXED:
+	break;
       case MEMMODEL_CONSUME:
-	return true;
-
+      case MEMMODEL_ACQUIRE:
+	fputs (".aq", file);
+	break;
       case MEMMODEL_RELEASE:
-      case MEMMODEL_RELAXED:
-	return false;
-
+	fputs (".rl", file);
+	break;
+      case MEMMODEL_ACQ_REL:
+      case MEMMODEL_SEQ_CST:
+	fputs (".aqrl", file);
+	break;
       default:
 	gcc_unreachable ();
     }
@@ -3658,8 +3660,7 @@ riscv_print_operand (FILE *file, rtx op, int letter)
       break;
 
     case 'A':
-      if (riscv_memmodel_needs_amo_acquire (model))
-	fputs (".aq", file);
+      riscv_print_amo_memory_ordering_suffix (file, model);
       break;
 
     case 'F':
-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 3/9] RISC-V: Eliminate %F specifier from riscv_print_operand() [PR 100265]
  2022-05-27  6:07 [PATCH v3 0/9] [RISC-V] Atomics improvements Christoph Muellner
  2022-05-27  6:07 ` [PATCH v3 1/9] RISC-V: Simplify memory model code [PR 100265] Christoph Muellner
  2022-05-27  6:07 ` [PATCH v3 2/9] RISC-V: Emit proper memory ordering suffixes for AMOs " Christoph Muellner
@ 2022-05-27  6:07 ` Christoph Muellner
  2022-05-27  6:07 ` [PATCH v3 4/9] RISC-V: Use STORE instead of AMOSWAP for atomic stores " Christoph Muellner
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Muellner @ 2022-05-27  6:07 UTC (permalink / raw)
  To: gcc-patches, Kito Cheng, Jim Wilson, Andrew Waterman,
	Philipp Tomsich, Christoph Muellner, Aaron Durbin,
	Patrick O'Neill, Vineet Gupta

A previous patch took care, that the proper memory ordering suffixes
for AMOs are emitted. Therefore there is no reason to keep the fence
generation mechanism for release operations.

    gcc/
        PR 100265
        * config/riscv/riscv.c (riscv_memmodel_needs_release_fence):
          Remove function.
        * config/riscv/riscv.c (riscv_print_operand): Remove
          %F format specifier.
        * config/riscv/sync.md: Remove %F format specifier uses.
---
 gcc/config/riscv/riscv.cc | 29 -----------------------------
 gcc/config/riscv/sync.md  | 16 ++++++++--------
 2 files changed, 8 insertions(+), 37 deletions(-)

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 983a567c69c..5bb22044ce9 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -3599,29 +3599,6 @@ riscv_print_amo_memory_ordering_suffix (FILE *file, const enum memmodel model)
     }
 }
 
-/* Return true if a FENCE should be emitted to before a memory access to
-   implement the release portion of memory model MODEL.  */
-
-static bool
-riscv_memmodel_needs_release_fence (const enum memmodel model)
-{
-  switch (model)
-    {
-      case MEMMODEL_ACQ_REL:
-      case MEMMODEL_SEQ_CST:
-      case MEMMODEL_RELEASE:
-	return true;
-
-      case MEMMODEL_ACQUIRE:
-      case MEMMODEL_CONSUME:
-      case MEMMODEL_RELAXED:
-	return false;
-
-      default:
-	gcc_unreachable ();
-    }
-}
-
 /* Implement TARGET_PRINT_OPERAND.  The RISCV-specific operand codes are:
 
    'h'	Print the high-part relocation associated with OP, after stripping
@@ -3629,7 +3606,6 @@ riscv_memmodel_needs_release_fence (const enum memmodel model)
    'R'	Print the low-part relocation associated with OP.
    'C'	Print the integer branch condition for comparison OP.
    'A'	Print the atomic operation suffix for memory model OP.
-   'F'	Print a FENCE if the memory model requires a release.
    'z'	Print x0 if OP is zero, otherwise print OP normally.
    'i'	Print i if the operand is not a register.
    'S'	Print shift-index of single-bit mask OP.
@@ -3663,11 +3639,6 @@ riscv_print_operand (FILE *file, rtx op, int letter)
       riscv_print_amo_memory_ordering_suffix (file, model);
       break;
 
-    case 'F':
-      if (riscv_memmodel_needs_release_fence (model))
-	fputs ("fence iorw,ow; ", file);
-      break;
-
     case 'i':
       if (code != REG)
         fputs ("i", file);
diff --git a/gcc/config/riscv/sync.md b/gcc/config/riscv/sync.md
index 86b41e6b00a..ddaeda0116d 100644
--- a/gcc/config/riscv/sync.md
+++ b/gcc/config/riscv/sync.md
@@ -65,7 +65,7 @@ (define_insn "atomic_store<mode>"
        (match_operand:SI 2 "const_int_operand")]      ;; model
       UNSPEC_ATOMIC_STORE))]
   "TARGET_ATOMIC"
-  "%F2amoswap.<amo>%A2 zero,%z1,%0"
+  "amoswap.<amo>%A2 zero,%z1,%0"
   [(set (attr "length") (const_int 8))])
 
 (define_insn "atomic_<atomic_optab><mode>"
@@ -76,8 +76,8 @@ (define_insn "atomic_<atomic_optab><mode>"
 	   (match_operand:SI 2 "const_int_operand")] ;; model
 	 UNSPEC_SYNC_OLD_OP))]
   "TARGET_ATOMIC"
-  "%F2amo<insn>.<amo>%A2 zero,%z1,%0"
-  [(set (attr "length") (const_int 8))])
+  "amo<insn>.<amo>%A2 zero,%z1,%0"
+)
 
 (define_insn "atomic_fetch_<atomic_optab><mode>"
   [(set (match_operand:GPR 0 "register_operand" "=&r")
@@ -89,8 +89,8 @@ (define_insn "atomic_fetch_<atomic_optab><mode>"
 	   (match_operand:SI 3 "const_int_operand")] ;; model
 	 UNSPEC_SYNC_OLD_OP))]
   "TARGET_ATOMIC"
-  "%F3amo<insn>.<amo>%A3 %0,%z2,%1"
-  [(set (attr "length") (const_int 8))])
+  "amo<insn>.<amo>%A3 %0,%z2,%1"
+)
 
 (define_insn "atomic_exchange<mode>"
   [(set (match_operand:GPR 0 "register_operand" "=&r")
@@ -101,8 +101,8 @@ (define_insn "atomic_exchange<mode>"
    (set (match_dup 1)
 	(match_operand:GPR 2 "register_operand" "0"))]
   "TARGET_ATOMIC"
-  "%F3amoswap.<amo>%A3 %0,%z2,%1"
-  [(set (attr "length") (const_int 8))])
+  "amoswap.<amo>%A3 %0,%z2,%1"
+)
 
 (define_insn "atomic_cas_value_strong<mode>"
   [(set (match_operand:GPR 0 "register_operand" "=&r")
@@ -115,7 +115,7 @@ (define_insn "atomic_cas_value_strong<mode>"
 	 UNSPEC_COMPARE_AND_SWAP))
    (clobber (match_scratch:GPR 6 "=&r"))]
   "TARGET_ATOMIC"
-  "%F5 1: lr.<amo>%A5 %0,%1; bne %0,%z2,1f; sc.<amo>%A4 %6,%z3,%1; bnez %6,1b; 1:"
+  "1: lr.<amo>%A5 %0,%1; bne %0,%z2,1f; sc.<amo>%A4 %6,%z3,%1; bnez %6,1b; 1:"
   [(set (attr "length") (const_int 20))])
 
 (define_expand "atomic_compare_and_swap<mode>"
-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 4/9] RISC-V: Use STORE instead of AMOSWAP for atomic stores [PR 100265]
  2022-05-27  6:07 [PATCH v3 0/9] [RISC-V] Atomics improvements Christoph Muellner
                   ` (2 preceding siblings ...)
  2022-05-27  6:07 ` [PATCH v3 3/9] RISC-V: Eliminate %F specifier from riscv_print_operand() " Christoph Muellner
@ 2022-05-27  6:07 ` Christoph Muellner
  2022-05-27  6:07 ` [PATCH v3 5/9] RISC-V: Emit fences according to chosen memory model " Christoph Muellner
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Muellner @ 2022-05-27  6:07 UTC (permalink / raw)
  To: gcc-patches, Kito Cheng, Jim Wilson, Andrew Waterman,
	Philipp Tomsich, Christoph Muellner, Aaron Durbin,
	Patrick O'Neill, Vineet Gupta

Using AMOSWAP as atomic store does not allow us to do sub-word accesses.
Further, it is not consistent with our atomic_load () implementation.
The benefit of AMOSWAP is that the resulting code sequence will be
smaller (comapred to FENCE+STORE), however, this does not weight
out for the lack of sub-word accesses.
Additionally, HW implementors have claimed that an optimal
implementation AMOSWAP is slightly more expensive than FENCE+STORE.
So let's use STORE instead of AMOSWAP.

    gcc/
        PR 100265
        * config/riscv/sync.md (atomic_store<mode>):
          Remove.
---
 gcc/config/riscv/sync.md | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/gcc/config/riscv/sync.md b/gcc/config/riscv/sync.md
index ddaeda0116d..86f4cef6af9 100644
--- a/gcc/config/riscv/sync.md
+++ b/gcc/config/riscv/sync.md
@@ -57,17 +57,6 @@ (define_insn "mem_thread_fence_1"
 
 ;; Atomic memory operations.
 
-;; Implement atomic stores with amoswap.  Fall back to fences for atomic loads.
-(define_insn "atomic_store<mode>"
-  [(set (match_operand:GPR 0 "memory_operand" "=A")
-    (unspec_volatile:GPR
-      [(match_operand:GPR 1 "reg_or_0_operand" "rJ")
-       (match_operand:SI 2 "const_int_operand")]      ;; model
-      UNSPEC_ATOMIC_STORE))]
-  "TARGET_ATOMIC"
-  "amoswap.<amo>%A2 zero,%z1,%0"
-  [(set (attr "length") (const_int 8))])
-
 (define_insn "atomic_<atomic_optab><mode>"
   [(set (match_operand:GPR 0 "memory_operand" "+A")
 	(unspec_volatile:GPR
-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 5/9] RISC-V: Emit fences according to chosen memory model [PR 100265]
  2022-05-27  6:07 [PATCH v3 0/9] [RISC-V] Atomics improvements Christoph Muellner
                   ` (3 preceding siblings ...)
  2022-05-27  6:07 ` [PATCH v3 4/9] RISC-V: Use STORE instead of AMOSWAP for atomic stores " Christoph Muellner
@ 2022-05-27  6:07 ` Christoph Muellner
  2022-05-27  6:07 ` [PATCH v3 6/9] RISC-V: Implement atomic_{load,store} " Christoph Muellner
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Muellner @ 2022-05-27  6:07 UTC (permalink / raw)
  To: gcc-patches, Kito Cheng, Jim Wilson, Andrew Waterman,
	Philipp Tomsich, Christoph Muellner, Aaron Durbin,
	Patrick O'Neill, Vineet Gupta

mem_thread_fence gets the desired memory model as operand.
Let's emit fences according to this value (as defined in section
    "Code Porting and Mapping Guidelines" of the unpriv spec).

    gcc/
        PR 100265
        * config/riscv/sync.md (mem_thread_fence):
          Emit fences according to given operand.
        * config/riscv/sync.md (mem_fence):
          Add INSNs for different fence flavours.
        * config/riscv/sync.md (mem_thread_fence_1):
          Remove.
---
 gcc/config/riscv/sync.md | 41 +++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/gcc/config/riscv/sync.md b/gcc/config/riscv/sync.md
index 86f4cef6af9..ae80f94f2e0 100644
--- a/gcc/config/riscv/sync.md
+++ b/gcc/config/riscv/sync.md
@@ -34,26 +34,41 @@ (define_code_attr atomic_optab
 ;; Memory barriers.
 
 (define_expand "mem_thread_fence"
-  [(match_operand:SI 0 "const_int_operand" "")] ;; model
+  [(match_operand:SI 0 "const_int_operand")] ;; model
   ""
 {
-  if (INTVAL (operands[0]) != MEMMODEL_RELAXED)
-    {
-      rtx mem = gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (Pmode));
-      MEM_VOLATILE_P (mem) = 1;
-      emit_insn (gen_mem_thread_fence_1 (mem, operands[0]));
-    }
+  enum memmodel model = memmodel_from_int (INTVAL (operands[0]));
+  if (!(is_mm_relaxed (model)))
+      emit_insn (gen_mem_fence (operands[0]));
   DONE;
 })
 
-;; Until the RISC-V memory model (hence its mapping from C++) is finalized,
-;; conservatively emit a full FENCE.
-(define_insn "mem_thread_fence_1"
+(define_expand "mem_fence"
+  [(set (match_dup 1)
+	(unspec:BLK [(match_dup 1) (match_operand:SI 0 "const_int_operand")]
+		    UNSPEC_MEMORY_BARRIER))]
+  ""
+{
+  operands[1] = gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (Pmode));
+  MEM_VOLATILE_P (operands[1]) = 1;
+})
+
+(define_insn "*mem_fence"
   [(set (match_operand:BLK 0 "" "")
-	(unspec:BLK [(match_dup 0)] UNSPEC_MEMORY_BARRIER))
-   (match_operand:SI 1 "const_int_operand" "")] ;; model
+	(unspec:BLK [(match_dup 0) (match_operand:SI 1 "const_int_operand")]
+		    UNSPEC_MEMORY_BARRIER))]
   ""
-  "fence\tiorw,iorw")
+{
+  enum memmodel model = memmodel_from_int (INTVAL (operands[1]));
+  if (is_mm_consume (model) || is_mm_acquire (model))
+    return "fence\tr, rw";
+  else if (is_mm_release (model))
+    return "fence\trw, w";
+  else if (is_mm_acq_rel (model))
+    return "fence.tso";
+  else
+    return "fence\trw, rw";
+})
 
 ;; Atomic memory operations.
 
-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 6/9] RISC-V: Implement atomic_{load,store} [PR 100265]
  2022-05-27  6:07 [PATCH v3 0/9] [RISC-V] Atomics improvements Christoph Muellner
                   ` (4 preceding siblings ...)
  2022-05-27  6:07 ` [PATCH v3 5/9] RISC-V: Emit fences according to chosen memory model " Christoph Muellner
@ 2022-05-27  6:07 ` Christoph Muellner
  2022-05-27  6:07 ` [PATCH v3 7/9] RISC-V: Model INSNs for LR and SC [PR 100266] Christoph Muellner
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Muellner @ 2022-05-27  6:07 UTC (permalink / raw)
  To: gcc-patches, Kito Cheng, Jim Wilson, Andrew Waterman,
	Philipp Tomsich, Christoph Muellner, Aaron Durbin,
	Patrick O'Neill, Vineet Gupta

A recent commit introduced a mechanism to emit proper fences
for RISC-V. Additionally, we already have emit_move_insn ().
Let's reuse this code and provide atomic_load<mode> and
atomic_store<mode> for RISC-V (as defined in section
"Code Porting and Mapping Guidelines" of the unpriv spec).
Note, that this works also for sub-word atomics.

    gcc/
        PR 100265
        * config/riscv/sync.md (atomic_load<mode>): New.
        * config/riscv/sync.md (atomic_store<mode>): New.
---
 gcc/config/riscv/sync.md | 41 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/gcc/config/riscv/sync.md b/gcc/config/riscv/sync.md
index ae80f94f2e0..9eb0dde9086 100644
--- a/gcc/config/riscv/sync.md
+++ b/gcc/config/riscv/sync.md
@@ -23,6 +23,7 @@ (define_c_enum "unspec" [
   UNSPEC_COMPARE_AND_SWAP
   UNSPEC_SYNC_OLD_OP
   UNSPEC_SYNC_EXCHANGE
+  UNSPEC_ATOMIC_LOAD
   UNSPEC_ATOMIC_STORE
   UNSPEC_MEMORY_BARRIER
 ])
@@ -72,6 +73,46 @@ (define_insn "*mem_fence"
 
 ;; Atomic memory operations.
 
+(define_expand "atomic_load<mode>"
+  [(set (match_operand:ANYI 0 "register_operand" "=r")
+    (unspec_volatile:ANYI
+      [(match_operand:ANYI 1 "memory_operand" "A")
+       (match_operand:SI 2 "const_int_operand")]      ;; model
+      UNSPEC_ATOMIC_LOAD))]
+  ""
+  {
+    rtx target = operands[0];
+    rtx mem = operands[1];
+    enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
+
+    if (is_mm_seq_cst (model))
+      emit_insn (gen_mem_fence (GEN_INT (MEMMODEL_SEQ_CST)));
+    emit_move_insn (target, mem);
+    if (is_mm_acquire (model) || is_mm_seq_cst (model))
+      emit_insn (gen_mem_fence (GEN_INT (MEMMODEL_ACQUIRE)));
+
+    DONE;
+})
+
+(define_expand "atomic_store<mode>"
+  [(set (match_operand:ANYI 0 "memory_operand" "=A")
+    (unspec_volatile:ANYI
+      [(match_operand:ANYI 1 "reg_or_0_operand" "rJ")
+       (match_operand:SI 2 "const_int_operand")]      ;; model
+      UNSPEC_ATOMIC_STORE))]
+  ""
+  {
+    rtx mem = operands[0];
+    rtx val = operands[1];
+    enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
+
+    if (is_mm_release (model) || is_mm_seq_cst (model))
+      emit_insn (gen_mem_fence (GEN_INT (MEMMODEL_RELEASE)));
+    emit_move_insn (mem, val);
+
+    DONE;
+})
+
 (define_insn "atomic_<atomic_optab><mode>"
   [(set (match_operand:GPR 0 "memory_operand" "+A")
 	(unspec_volatile:GPR
-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 7/9] RISC-V: Model INSNs for LR and SC [PR 100266]
  2022-05-27  6:07 [PATCH v3 0/9] [RISC-V] Atomics improvements Christoph Muellner
                   ` (5 preceding siblings ...)
  2022-05-27  6:07 ` [PATCH v3 6/9] RISC-V: Implement atomic_{load,store} " Christoph Muellner
@ 2022-05-27  6:07 ` Christoph Muellner
  2022-05-27  6:07 ` [PATCH v3 8/9] RISC-V: Add s.ext-consuming " Christoph Muellner
  2022-05-27  6:07 ` [PATCH v3 9/9] RISC-V: Introduce predicate "riscv_sync_memory_operand" " Christoph Muellner
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Muellner @ 2022-05-27  6:07 UTC (permalink / raw)
  To: gcc-patches, Kito Cheng, Jim Wilson, Andrew Waterman,
	Philipp Tomsich, Christoph Muellner, Aaron Durbin,
	Patrick O'Neill, Vineet Gupta

In order to emit LR/SC sequences, let's provide INSNs, which
take care of memory ordering constraints.

    gcc/
        PR 100266
        * config/rsicv/sync.md (UNSPEC_LOAD_RESERVED): New.
        * config/rsicv/sync.md (UNSPEC_STORE_CONDITIONAL): New.
        * config/riscv/sync.md (riscv_load_reserved): New.
        * config/riscv/sync.md (riscv_store_conditional): New.
---
 gcc/config/riscv/sync.md | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/gcc/config/riscv/sync.md b/gcc/config/riscv/sync.md
index 9eb0dde9086..3494683947e 100644
--- a/gcc/config/riscv/sync.md
+++ b/gcc/config/riscv/sync.md
@@ -26,6 +26,8 @@ (define_c_enum "unspec" [
   UNSPEC_ATOMIC_LOAD
   UNSPEC_ATOMIC_STORE
   UNSPEC_MEMORY_BARRIER
+  UNSPEC_LOAD_RESERVED
+  UNSPEC_STORE_CONDITIONAL
 ])
 
 (define_code_iterator any_atomic [plus ior xor and])
@@ -113,6 +115,28 @@ (define_expand "atomic_store<mode>"
     DONE;
 })
 
+(define_insn "@riscv_load_reserved<mode>"
+  [(set (match_operand:GPR 0 "register_operand" "=r")
+    (unspec_volatile:GPR
+      [(match_operand:GPR 1 "memory_operand" "A")
+       (match_operand:SI 2 "const_int_operand")]      ;; model
+      UNSPEC_LOAD_RESERVED))]
+  "TARGET_ATOMIC"
+  "lr.<amo>%A2 %0, %1"
+)
+
+(define_insn "@riscv_store_conditional<mode>"
+  [(set (match_operand:GPR 0 "register_operand" "=&r")
+    (unspec_volatile:GPR [(const_int 0)] UNSPEC_STORE_CONDITIONAL))
+   (set (match_operand:GPR 1 "memory_operand" "=A")
+    (unspec_volatile:GPR
+      [(match_operand:GPR 2 "reg_or_0_operand" "rJ")
+       (match_operand:SI 3 "const_int_operand")]      ;; model
+      UNSPEC_STORE_CONDITIONAL))]
+  "TARGET_ATOMIC"
+  "sc.<amo>%A3 %0, %z2, %1"
+)
+
 (define_insn "atomic_<atomic_optab><mode>"
   [(set (match_operand:GPR 0 "memory_operand" "+A")
 	(unspec_volatile:GPR
-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 8/9] RISC-V: Add s.ext-consuming INSNs for LR and SC [PR 100266]
  2022-05-27  6:07 [PATCH v3 0/9] [RISC-V] Atomics improvements Christoph Muellner
                   ` (6 preceding siblings ...)
  2022-05-27  6:07 ` [PATCH v3 7/9] RISC-V: Model INSNs for LR and SC [PR 100266] Christoph Muellner
@ 2022-05-27  6:07 ` Christoph Muellner
  2022-05-27  6:07 ` [PATCH v3 9/9] RISC-V: Introduce predicate "riscv_sync_memory_operand" " Christoph Muellner
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Muellner @ 2022-05-27  6:07 UTC (permalink / raw)
  To: gcc-patches, Kito Cheng, Jim Wilson, Andrew Waterman,
	Philipp Tomsich, Christoph Muellner, Aaron Durbin,
	Patrick O'Neill, Vineet Gupta

The current model of the LR and SC INSNs requires a sign-extension
to use the generated SImode value for conditional branches, which
only operate on XLEN registers.
However, the sign-extension is actually not required in both cases,
therefore this patch introduces additional INSNs that consume
the sign-extension.

Rationale:
The sign-extension of the loaded value of a LR.W is specified as
sign-extended. Therefore, a sign-extension is not required.
The sign-extension of the success value a SC.W is specified as
non-zero. As sign-extended non-zero value remains non-zero,
therefore the sign-extension is not required.

    gcc/
        PR 100266
        * config/riscv/sync.md (riscv_load_reserved): New.
        * config/riscv/sync.md (riscv_store_conditional): New.
---
 gcc/config/riscv/sync.md | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/gcc/config/riscv/sync.md b/gcc/config/riscv/sync.md
index 3494683947e..66548bf891b 100644
--- a/gcc/config/riscv/sync.md
+++ b/gcc/config/riscv/sync.md
@@ -125,6 +125,21 @@ (define_insn "@riscv_load_reserved<mode>"
   "lr.<amo>%A2 %0, %1"
 )
 
+;; This pattern allows to consume a sign-extension of the loaded value.
+;; This is legal, because the specification of LR.W defines the loaded
+;; value to be sign-extended.
+
+(define_insn "riscv_load_reserved"
+  [(set (match_operand:DI 0 "register_operand" "=r")
+    (sign_extend:DI
+      (unspec_volatile:SI
+	[(match_operand:SI 1 "memory_operand" "A")
+	 (match_operand:SI 2 "const_int_operand")]      ;; model
+	UNSPEC_LOAD_RESERVED)))]
+  "TARGET_ATOMIC && TARGET_64BIT"
+  "lr.w%A2 %0, %1"
+)
+
 (define_insn "@riscv_store_conditional<mode>"
   [(set (match_operand:GPR 0 "register_operand" "=&r")
     (unspec_volatile:GPR [(const_int 0)] UNSPEC_STORE_CONDITIONAL))
@@ -137,6 +152,25 @@ (define_insn "@riscv_store_conditional<mode>"
   "sc.<amo>%A3 %0, %z2, %1"
 )
 
+;; This pattern allows to consume a sign-extension of the success
+;; value of SC.W, which can then be used for instructions which
+;; require values of XLEN-size (e.g. conditional branches).
+;; This is legal, because any non-zero value remains non-zero
+;; after sign-extension.
+
+(define_insn "riscv_store_conditional"
+  [(set (match_operand:DI 0 "register_operand" "=&r")
+    (sign_extend:DI
+      (unspec_volatile:SI [(const_int 0)] UNSPEC_STORE_CONDITIONAL)))
+   (set (match_operand:SI 1 "memory_operand" "=A")
+    (unspec_volatile:SI
+      [(match_operand:SI 2 "reg_or_0_operand" "rJ")
+       (match_operand:SI 3 "const_int_operand")]      ;; model
+      UNSPEC_STORE_CONDITIONAL))]
+  "TARGET_ATOMIC && TARGET_64BIT"
+  "sc.w%A3 %0, %z2, %1"
+)
+
 (define_insn "atomic_<atomic_optab><mode>"
   [(set (match_operand:GPR 0 "memory_operand" "+A")
 	(unspec_volatile:GPR
-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 9/9] RISC-V: Introduce predicate "riscv_sync_memory_operand" [PR 100266]
  2022-05-27  6:07 [PATCH v3 0/9] [RISC-V] Atomics improvements Christoph Muellner
                   ` (7 preceding siblings ...)
  2022-05-27  6:07 ` [PATCH v3 8/9] RISC-V: Add s.ext-consuming " Christoph Muellner
@ 2022-05-27  6:07 ` Christoph Muellner
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Muellner @ 2022-05-27  6:07 UTC (permalink / raw)
  To: gcc-patches, Kito Cheng, Jim Wilson, Andrew Waterman,
	Philipp Tomsich, Christoph Muellner, Aaron Durbin,
	Patrick O'Neill, Vineet Gupta

Atomic instructions require zero-offset memory addresses.
If we allow all addresses, the nonzero-offset addresses will
be prepared in an extra register in an extra instruction before
the actual atomic instruction.

This patch introduces the predicate "riscv_sync_memory_operand",
which restricts the memory operand to be suitable for atomic
instructions.

    gcc/
        PR 100266
        * config/riscv/sync.md (riscv_sync_memory_operand): New.
        * config/riscv/sync.md (riscv_load_reserved): Use new predicate.
        * config/riscv/sync.md (riscv_store_conditional): Likewise.
        * config/riscv/sync.md (atomic_<atomic_optab>): Likewise.
        * config/riscv/sync.md (atomic_fetch_<atomic_optab>): Likewise.
        * config/riscv/sync.md (atomic_exchange): Likewise.
        * config/riscv/sync.md (atomic_compare_and_swap): Likewise.
---
 gcc/config/riscv/sync.md | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/gcc/config/riscv/sync.md b/gcc/config/riscv/sync.md
index 66548bf891b..8f184d8bbb4 100644
--- a/gcc/config/riscv/sync.md
+++ b/gcc/config/riscv/sync.md
@@ -30,6 +30,10 @@ (define_c_enum "unspec" [
   UNSPEC_STORE_CONDITIONAL
 ])
 
+(define_predicate "riscv_sync_memory_operand"
+  (and (match_operand 0 "memory_operand")
+       (match_code "reg" "0")))
+
 (define_code_iterator any_atomic [plus ior xor and])
 (define_code_attr atomic_optab
   [(plus "add") (ior "or") (xor "xor") (and "and")])
@@ -118,7 +122,7 @@ (define_expand "atomic_store<mode>"
 (define_insn "@riscv_load_reserved<mode>"
   [(set (match_operand:GPR 0 "register_operand" "=r")
     (unspec_volatile:GPR
-      [(match_operand:GPR 1 "memory_operand" "A")
+      [(match_operand:GPR 1 "riscv_sync_memory_operand" "A")
        (match_operand:SI 2 "const_int_operand")]      ;; model
       UNSPEC_LOAD_RESERVED))]
   "TARGET_ATOMIC"
@@ -133,7 +137,7 @@ (define_insn "riscv_load_reserved"
   [(set (match_operand:DI 0 "register_operand" "=r")
     (sign_extend:DI
       (unspec_volatile:SI
-	[(match_operand:SI 1 "memory_operand" "A")
+	[(match_operand:SI 1 "riscv_sync_memory_operand" "A")
 	 (match_operand:SI 2 "const_int_operand")]      ;; model
 	UNSPEC_LOAD_RESERVED)))]
   "TARGET_ATOMIC && TARGET_64BIT"
@@ -143,7 +147,7 @@ (define_insn "riscv_load_reserved"
 (define_insn "@riscv_store_conditional<mode>"
   [(set (match_operand:GPR 0 "register_operand" "=&r")
     (unspec_volatile:GPR [(const_int 0)] UNSPEC_STORE_CONDITIONAL))
-   (set (match_operand:GPR 1 "memory_operand" "=A")
+   (set (match_operand:GPR 1 "riscv_sync_memory_operand" "=A")
     (unspec_volatile:GPR
       [(match_operand:GPR 2 "reg_or_0_operand" "rJ")
        (match_operand:SI 3 "const_int_operand")]      ;; model
@@ -162,7 +166,7 @@ (define_insn "riscv_store_conditional"
   [(set (match_operand:DI 0 "register_operand" "=&r")
     (sign_extend:DI
       (unspec_volatile:SI [(const_int 0)] UNSPEC_STORE_CONDITIONAL)))
-   (set (match_operand:SI 1 "memory_operand" "=A")
+   (set (match_operand:SI 1 "riscv_sync_memory_operand" "=A")
     (unspec_volatile:SI
       [(match_operand:SI 2 "reg_or_0_operand" "rJ")
        (match_operand:SI 3 "const_int_operand")]      ;; model
@@ -172,7 +176,7 @@ (define_insn "riscv_store_conditional"
 )
 
 (define_insn "atomic_<atomic_optab><mode>"
-  [(set (match_operand:GPR 0 "memory_operand" "+A")
+  [(set (match_operand:GPR 0 "riscv_sync_memory_operand" "+A")
 	(unspec_volatile:GPR
 	  [(any_atomic:GPR (match_dup 0)
 		     (match_operand:GPR 1 "reg_or_0_operand" "rJ"))
@@ -184,7 +188,7 @@ (define_insn "atomic_<atomic_optab><mode>"
 
 (define_insn "atomic_fetch_<atomic_optab><mode>"
   [(set (match_operand:GPR 0 "register_operand" "=&r")
-	(match_operand:GPR 1 "memory_operand" "+A"))
+	(match_operand:GPR 1 "riscv_sync_memory_operand" "+A"))
    (set (match_dup 1)
 	(unspec_volatile:GPR
 	  [(any_atomic:GPR (match_dup 1)
@@ -198,7 +202,7 @@ (define_insn "atomic_fetch_<atomic_optab><mode>"
 (define_insn "atomic_exchange<mode>"
   [(set (match_operand:GPR 0 "register_operand" "=&r")
 	(unspec_volatile:GPR
-	  [(match_operand:GPR 1 "memory_operand" "+A")
+	  [(match_operand:GPR 1 "riscv_sync_memory_operand" "+A")
 	   (match_operand:SI 3 "const_int_operand")] ;; model
 	  UNSPEC_SYNC_EXCHANGE))
    (set (match_dup 1)
@@ -222,14 +226,14 @@ (define_insn "atomic_cas_value_strong<mode>"
   [(set (attr "length") (const_int 20))])
 
 (define_expand "atomic_compare_and_swap<mode>"
-  [(match_operand:SI 0 "register_operand" "")   ;; bool output
-   (match_operand:GPR 1 "register_operand" "")  ;; val output
-   (match_operand:GPR 2 "memory_operand" "")    ;; memory
-   (match_operand:GPR 3 "reg_or_0_operand" "")  ;; expected value
-   (match_operand:GPR 4 "reg_or_0_operand" "")  ;; desired value
-   (match_operand:SI 5 "const_int_operand" "")  ;; is_weak
-   (match_operand:SI 6 "const_int_operand" "")  ;; mod_s
-   (match_operand:SI 7 "const_int_operand" "")] ;; mod_f
+  [(match_operand:SI 0 "register_operand" "")           ;; bool output
+   (match_operand:GPR 1 "register_operand" "")          ;; val output
+   (match_operand:GPR 2 "riscv_sync_memory_operand" "") ;; memory
+   (match_operand:GPR 3 "reg_or_0_operand" "")          ;; expected value
+   (match_operand:GPR 4 "reg_or_0_operand" "")          ;; desired value
+   (match_operand:SI 5 "const_int_operand" "")          ;; is_weak
+   (match_operand:SI 6 "const_int_operand" "")          ;; mod_s
+   (match_operand:SI 7 "const_int_operand" "")]         ;; mod_f
   "TARGET_ATOMIC"
 {
   emit_insn (gen_atomic_cas_value_strong<mode> (operands[1], operands[2],
-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-05-27  6:07 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-27  6:07 [PATCH v3 0/9] [RISC-V] Atomics improvements Christoph Muellner
2022-05-27  6:07 ` [PATCH v3 1/9] RISC-V: Simplify memory model code [PR 100265] Christoph Muellner
2022-05-27  6:07 ` [PATCH v3 2/9] RISC-V: Emit proper memory ordering suffixes for AMOs " Christoph Muellner
2022-05-27  6:07 ` [PATCH v3 3/9] RISC-V: Eliminate %F specifier from riscv_print_operand() " Christoph Muellner
2022-05-27  6:07 ` [PATCH v3 4/9] RISC-V: Use STORE instead of AMOSWAP for atomic stores " Christoph Muellner
2022-05-27  6:07 ` [PATCH v3 5/9] RISC-V: Emit fences according to chosen memory model " Christoph Muellner
2022-05-27  6:07 ` [PATCH v3 6/9] RISC-V: Implement atomic_{load,store} " Christoph Muellner
2022-05-27  6:07 ` [PATCH v3 7/9] RISC-V: Model INSNs for LR and SC [PR 100266] Christoph Muellner
2022-05-27  6:07 ` [PATCH v3 8/9] RISC-V: Add s.ext-consuming " Christoph Muellner
2022-05-27  6:07 ` [PATCH v3 9/9] RISC-V: Introduce predicate "riscv_sync_memory_operand" " Christoph Muellner

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