[v5,04/11] RISC-V: Enforce atomic compare_exchange SEQ_CST

Message ID 20230427162301.1151333-5-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
  This patch enforces SEQ_CST for atomic compare_exchange ops.

Replace Fence/LR.aq/SC.aq pairs with SEQ_CST LR.aqrl/SC.rl pairs
recommended by table A.6 of the ISA manual.

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

gcc/ChangeLog:

	* config/riscv/sync.md: Change FENCE/LR.aq/SC.aq into
	sequentially consistent LR.aqrl/SC.rl pair.

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

Comments

Jeff Law April 28, 2023, 5:23 p.m. UTC | #1
On 4/27/23 10:22, Patrick O'Neill wrote:
> This patch enforces SEQ_CST for atomic compare_exchange ops.
> 
> Replace Fence/LR.aq/SC.aq pairs with SEQ_CST LR.aqrl/SC.rl pairs
> recommended by table A.6 of the ISA manual.
> 
> 2023-04-27 Patrick O'Neill <patrick@rivosinc.com>
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/sync.md: Change FENCE/LR.aq/SC.aq into
> 	sequentially consistent LR.aqrl/SC.rl pair.
OK.  Note that generally you should note which pattern you're changing 
in a ChangeLog entry, similar to how we note the function being changed. 
  So something like this might be better:

	* config/riscv/sync.md (atomic_cas_value_strong<mode>): ...

Jeff
  
Patrick O'Neill May 2, 2023, 8:15 p.m. UTC | #2
On 4/28/23 10:23, Jeff Law wrote:
>
>
> On 4/27/23 10:22, Patrick O'Neill wrote:
>> This patch enforces SEQ_CST for atomic compare_exchange ops.
>>
>> Replace Fence/LR.aq/SC.aq pairs with SEQ_CST LR.aqrl/SC.rl pairs
>> recommended by table A.6 of the ISA manual.
>>
>> 2023-04-27 Patrick O'Neill <patrick@rivosinc.com>
>>
>> gcc/ChangeLog:
>>
>>     * config/riscv/sync.md: Change FENCE/LR.aq/SC.aq into
>>     sequentially consistent LR.aqrl/SC.rl pair.
> OK.  Note that generally you should note which pattern you're changing 
> in a ChangeLog entry, similar to how we note the function being 
> changed.  So something like this might be better:
>
>     * config/riscv/sync.md (atomic_cas_value_strong<mode>): ...
>
> Jeff

Edited ChangeLog and committed:

gcc/ChangeLog:

     * config/riscv/sync.md (atomic_cas_value_strong<mode>): Change
     FENCE/LR.aq/SC.aq into sequentially consistent LR.aqrl/SC.rl
     pair.

Patrick
  

Patch

diff --git a/gcc/config/riscv/sync.md b/gcc/config/riscv/sync.md
index 0c83ef04607..5620d6ffa58 100644
--- a/gcc/config/riscv/sync.md
+++ b/gcc/config/riscv/sync.md
@@ -297,9 +297,16 @@ 
 	 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:"
+  {
+    return "1:\;"
+	   "lr.<amo>.aqrl\t%0,%1\;"
+	   "bne\t%0,%z2,1f\;"
+	   "sc.<amo>.rl\t%6,%z3,%1\;"
+	   "bnez\t%6,1b\;"
+	   "1:";
+  }
   [(set_attr "type" "atomic")
-   (set (attr "length") (const_int 20))])
+   (set (attr "length") (const_int 16))])
 
 (define_expand "atomic_compare_and_swap<mode>"
   [(match_operand:SI 0 "register_operand" "")   ;; bool output