diff mbox series

[v5,07/11] RISC-V: Eliminate AMO op fences

Message ID 20230427162301.1151333-8-patrick@rivosinc.com
State Accepted
Headers
Series RISC-V: Implement ISA Manual Table A.6 Mappings |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Patrick O'Neill April 27, 2023, 4:22 p.m. UTC 
  Atomic operations with the appropriate bits set already enfore release
semantics. Remove unnecessary release fences from atomic ops.

This change brings AMO ops in line with table A.6 of the ISA manual.

2023-04-27 Patrick O'Neill <patrick@rivosinc.com>

gcc/ChangeLog:

	* config/riscv/riscv.cc
	(riscv_memmodel_needs_amo_release): Change function name.
	(riscv_print_operand): Remove unneeded %F case.
	* config/riscv/sync.md: Remove unneeded fences.

Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
---
 gcc/config/riscv/riscv.cc | 16 +++++-----------
 gcc/config/riscv/sync.md  | 12 ++++++------
 2 files changed, 11 insertions(+), 17 deletions(-)

Comments

Jeff Law April 28, 2023, 5:43 p.m. UTC | #1 
On 4/27/23 10:22, Patrick O'Neill wrote:
> Atomic operations with the appropriate bits set already enfore release
> semantics. Remove unnecessary release fences from atomic ops.
> 
> This change brings AMO ops in line with table A.6 of the ISA manual.
> 
> 2023-04-27 Patrick O'Neill <patrick@rivosinc.com>
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/riscv.cc
> 	(riscv_memmodel_needs_amo_release): Change function name.
> 	(riscv_print_operand): Remove unneeded %F case.
> 	* config/riscv/sync.md: Remove unneeded fences.
OK.  Though note this depends on a resolution of patch #6.  You could 
potentially leave the %F support in riscv_print_operand and install the 
rest of this patch while we settle the question around #6.

Jeff
Patrick O'Neill May 2, 2023, 8:19 p.m. UTC | #2 
On 4/28/23 10:43, Jeff Law wrote:
>
>
> On 4/27/23 10:22, Patrick O'Neill wrote:
>> Atomic operations with the appropriate bits set already enfore release
>> semantics. Remove unnecessary release fences from atomic ops.
>>
>> This change brings AMO ops in line with table A.6 of the ISA manual.
>>
>> 2023-04-27 Patrick O'Neill <patrick@rivosinc.com>
>>
>> gcc/ChangeLog:
>>
>>     * config/riscv/riscv.cc
>>     (riscv_memmodel_needs_amo_release): Change function name.
>>     (riscv_print_operand): Remove unneeded %F case.
>>     * config/riscv/sync.md: Remove unneeded fences.
> OK.  Though note this depends on a resolution of patch #6.  You could 
> potentially leave the %F support in riscv_print_operand and install 
> the rest of this patch while we settle the question around #6.
>
> Jeff

Committed.

Patrick
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index d46781d8981..9eba03ac189 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -4312,11 +4312,11 @@  riscv_memmodel_needs_amo_acquire (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.  */
+/* Return true if the .RL suffix should be added to an AMO to implement the
+   release portion of memory model MODEL.  */
 
 static bool
-riscv_memmodel_needs_release_fence (enum memmodel model)
+riscv_memmodel_needs_amo_release (enum memmodel model)
 {
   switch (model)
     {
@@ -4342,7 +4342,6 @@  riscv_memmodel_needs_release_fence (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.
@@ -4504,19 +4503,14 @@  riscv_print_operand (FILE *file, rtx op, int letter)
 
     case 'A':
       if (riscv_memmodel_needs_amo_acquire (model)
-	  && riscv_memmodel_needs_release_fence (model))
+	  && riscv_memmodel_needs_amo_release (model))
 	fputs (".aqrl", file);
       else if (riscv_memmodel_needs_amo_acquire (model))
 	fputs (".aq", file);
-      else if (riscv_memmodel_needs_release_fence (model))
+      else if (riscv_memmodel_needs_amo_release (model))
 	fputs (".rl", file);
       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 1acb78a9ae4..9a3b57bd09f 100644
--- a/gcc/config/riscv/sync.md
+++ b/gcc/config/riscv/sync.md
@@ -91,9 +91,9 @@ 
 	   (match_operand:SI 2 "const_int_operand")] ;; model
 	 UNSPEC_SYNC_OLD_OP))]
   "TARGET_ATOMIC"
-  "%F2amo<insn>.<amo>%A2 zero,%z1,%0"
+  "amo<insn>.<amo>%A2\tzero,%z1,%0"
   [(set_attr "type" "atomic")
-   (set (attr "length") (const_int 8))])
+   (set (attr "length") (const_int 4))])
 
 (define_insn "atomic_fetch_<atomic_optab><mode>"
   [(set (match_operand:GPR 0 "register_operand" "=&r")
@@ -105,9 +105,9 @@ 
 	   (match_operand:SI 3 "const_int_operand")] ;; model
 	 UNSPEC_SYNC_OLD_OP))]
   "TARGET_ATOMIC"
-  "%F3amo<insn>.<amo>%A3 %0,%z2,%1"
+  "amo<insn>.<amo>%A3\t%0,%z2,%1"
   [(set_attr "type" "atomic")
-   (set (attr "length") (const_int 8))])
+   (set (attr "length") (const_int 4))])
 
 (define_insn "subword_atomic_fetch_strong_<atomic_optab>"
   [(set (match_operand:SI 0 "register_operand" "=&r")		   ;; old value at mem
@@ -247,9 +247,9 @@ 
    (set (match_dup 1)
 	(match_operand:GPR 2 "register_operand" "0"))]
   "TARGET_ATOMIC"
-  "%F3amoswap.<amo>%A3 %0,%z2,%1"
+  "amoswap.<amo>%A3\t%0,%z2,%1"
   [(set_attr "type" "atomic")
-   (set (attr "length") (const_int 8))])
+   (set (attr "length") (const_int 4))])
 
 (define_expand "atomic_exchange<mode>"
   [(match_operand:SHORT 0 "register_operand") ;; old value at mem