From: Shreya Munnangi Date: Tue, 4 Aug 2026 13:16:37 +0000 (-0600) Subject: [RISC-V][PR target/108031] Expose address computations for atomic memory operations X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=097d4a9311cfc4c41cd885ee63285ef072a0e8fa;p=thirdparty%2Fgcc.git [RISC-V][PR target/108031] Expose address computations for atomic memory operations This is a patch from Shreya that takes a step towards fixing pr108031. Access to objects in static storage requires a high/lo_sum pair on RISC-V. Often, but not always, the lo_sum expression can be folded into the actual memory reference. One of the common cases where it can *not* fold in is atomic memory operations. So if (for example) we access nearby fields in a structure in static memory we'll often see lui a5,%hi(s) li a4,1 addi a5,a5,%lo(s) amoadd.w a0,a4,0(a5) lui a5,%hi(s+4) <-- this should be: addi a5, a5, 4 addi a5,a5,%lo(s+4) <-- this should be removed amoadd.w a3,a4,0(a5) add a0,a0,a3 We'd like to replace the second lui+addi pair with a single addi. That's normally a job for CSE, but due to implementation details of the RISC-V atomics we're failing to even expose those addresses to CSE. The core issue is the predicates on these instructions are wider than the constraints and as a result the lo_sum stays folded into the memory reference until LRA realizes the constraints don't match and the lo_sum part of the address computation gets reloaded. The fix is straightforward. Tighten the operand predicates. RISC-V only allows simple memory indirect operands for these instructions, yet surprisingly we didn't have a predicate for that kind of address. This patch adds an appropriate predicate, then uses it on the dozen or so relevant patterns/expanders. That's enough to expose the address calculation to CSE, schedulers, etc. In my (Jeff's) opinion the patch stands as an independent improvement, even though it doesn't fix 108031. The next (and final) step to fix 108031 will most likely be a costing model fix. ie, CSE will do the right thing with the addresses are fully exposed, but it rejects the changes because the RISC-V cost model is broken. This has been bootstrapped and regression tested on the c920. K3 bootstrap & regression test was just about done when a cat went crazy behind my desk and ultimately dislodged the power cable from the wall. It's restarted, but results are now 9 hours out :( riscv32-elf and riscv64-elf both worked fine, of course. I'm pushing this to the trunk of Shreya's behalf. PR target/108031 gcc * config/riscv/predicates.md (riscv_atomic_memory_operand): New predicate. * config/riscv/sync.md ( pattern and expander): Use riscv_atomic_memory_operand. (amo_atomic_): Likewise. (lrsc_atomic_): Likewise. (atomic_fetch_): Likewise. (lrsc_atomic_fetch_): Likewise. (subword_atomic_fetch_strong_): Likewise. (atomic_fetch_nand): Likewise. (subword_atomic_fetch_strong_nand): Likewise. (zabha_atomic_fetch_): Likewise. (lrsc_atomic_fetch_): Likewise. (atomic_exchange): Likewise. (amo_atomic_exchange): Likewise. (amo_atomic_exchange_extended): Likewise. (lrsc_atomic_exchange): Likewise. (subword_atomic_exchange_strong): Likewise. (zacas_atomic_cas_value_strong): Likewise. (zalrsc_atomic_cas_value_strong): Likewise. (subword_atomic_cas_strong): Likewise. (atomic_test_and_set): Likewise. gcc/testsuite * gcc.target/riscv/pr108031.c: New test. --- diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md index df1a76049f4..3ee9042b949 100644 --- a/gcc/config/riscv/predicates.md +++ b/gcc/config/riscv/predicates.md @@ -790,3 +790,7 @@ (define_predicate "ads_extract_size_imm_di" (and (match_code "const_int") (match_test "IN_RANGE (INTVAL (op), 1, 64)"))) + +(define_predicate "riscv_atomic_memory_operand" + (and (match_code "mem") + (match_test "register_operand (XEXP (op, 0), Pmode)"))) diff --git a/gcc/config/riscv/sync.md b/gcc/config/riscv/sync.md index a046519c835..0d983ce6024 100644 --- a/gcc/config/riscv/sync.md +++ b/gcc/config/riscv/sync.md @@ -94,7 +94,7 @@ ;; AMO ops (define_insn "atomic_" - [(set (match_operand:SHORT 0 "memory_operand" "+A") + [(set (match_operand:SHORT 0 "riscv_atomic_memory_operand" "+A") (unspec_volatile:SHORT [(any_atomic:SHORT (match_dup 0) (match_operand:SHORT 1 "reg_or_0_operand" "rJ")) @@ -106,7 +106,7 @@ (set (attr "length") (const_int 4))]) (define_expand "atomic_" - [(any_atomic:GPR (match_operand:GPR 0 "memory_operand") ;; mem location + [(any_atomic:GPR (match_operand:GPR 0 "riscv_atomic_memory_operand") ;; mem location (match_operand:GPR 1 "reg_or_0_operand")) ;; value for op (match_operand:SI 2 "const_int_operand")] ;; model "TARGET_ZAAMO || TARGET_ZALRSC" @@ -121,7 +121,7 @@ }) (define_insn "amo_atomic_" - [(set (match_operand:GPR 0 "memory_operand" "+A") + [(set (match_operand:GPR 0 "riscv_atomic_memory_operand" "+A") (unspec_volatile:GPR [(any_atomic:GPR (match_dup 0) (match_operand:GPR 1 "reg_or_0_operand" "rJ")) @@ -133,7 +133,7 @@ (set (attr "length") (const_int 4))]) (define_insn "lrsc_atomic_" - [(set (match_operand:GPR 0 "memory_operand" "+A") + [(set (match_operand:GPR 0 "riscv_atomic_memory_operand" "+A") (unspec_volatile:GPR [(any_atomic:GPR (match_dup 0) (match_operand:GPR 1 "reg_or_0_operand" "rJ")) @@ -155,7 +155,7 @@ (define_expand "atomic_fetch_" [(match_operand:GPR 0 "register_operand") ;; old value at mem - (any_atomic:GPR (match_operand:GPR 1 "memory_operand") ;; mem location + (any_atomic:GPR (match_operand:GPR 1 "riscv_atomic_memory_operand") ;; mem location (match_operand:GPR 2 "reg_or_0_operand")) ;; value for op (match_operand:SI 3 "const_int_operand")] ;; model "TARGET_ZAAMO || TARGET_ZALRSC" @@ -171,7 +171,7 @@ (define_insn "amo_atomic_fetch_" [(set (match_operand:GPR 0 "register_operand" "=&r") - (match_operand:GPR 1 "memory_operand" "+A")) + (match_operand:GPR 1 "riscv_atomic_memory_operand" "+A")) (set (match_dup 1) (unspec_volatile:GPR [(any_atomic:GPR (match_dup 1) @@ -185,7 +185,7 @@ (define_insn "lrsc_atomic_fetch_" [(set (match_operand:GPR 0 "register_operand" "=&r") - (match_operand:GPR 1 "memory_operand" "+A")) + (match_operand:GPR 1 "riscv_atomic_memory_operand" "+A")) (set (match_dup 1) (unspec_volatile:GPR [(any_atomic:GPR (match_dup 1) @@ -206,7 +206,7 @@ (define_insn "subword_atomic_fetch_strong_" [(set (match_operand:SI 0 "register_operand" "=&r") ;; old value at mem - (match_operand:SI 1 "memory_operand" "+A")) ;; mem location + (match_operand:SI 1 "riscv_atomic_memory_operand" "+A")) ;; mem location (set (match_dup 1) (unspec_volatile:SI [(any_atomic:SI (match_dup 1) @@ -233,7 +233,7 @@ (define_expand "atomic_fetch_nand" [(match_operand:SHORT 0 "register_operand") ;; old value at mem - (not:SHORT (and:SHORT (match_operand:SHORT 1 "memory_operand") ;; mem location + (not:SHORT (and:SHORT (match_operand:SHORT 1 "riscv_atomic_memory_operand") ;; mem location (match_operand:SHORT 2 "reg_or_0_operand"))) ;; value for op (match_operand:SI 3 "const_int_operand")] ;; model "TARGET_ZALRSC && TARGET_INLINE_SUBWORD_ATOMIC" @@ -273,7 +273,7 @@ (define_insn "subword_atomic_fetch_strong_nand" [(set (match_operand:SI 0 "register_operand" "=&r") ;; old value at mem - (match_operand:SI 1 "memory_operand" "+A")) ;; mem location + (match_operand:SI 1 "riscv_atomic_memory_operand" "+A")) ;; mem location (set (match_dup 1) (unspec_volatile:SI [(not:SI (and:SI (match_dup 1) @@ -301,7 +301,7 @@ (define_expand "atomic_fetch_" [(match_operand:SHORT 0 "register_operand") ;; old value at mem - (any_atomic:SHORT (match_operand:SHORT 1 "memory_operand") ;; mem location + (any_atomic:SHORT (match_operand:SHORT 1 "riscv_atomic_memory_operand") ;; mem location (match_operand:SHORT 2 "reg_or_0_operand")) ;; value for op (match_operand:SI 3 "const_int_operand")] ;; model "(TARGET_ZALRSC && TARGET_INLINE_SUBWORD_ATOMIC) || TARGET_ZABHA" @@ -317,7 +317,7 @@ (define_insn "zabha_atomic_fetch_" [(set (match_operand:SHORT 0 "register_operand" "=&r") - (match_operand:SHORT 1 "memory_operand" "+A")) + (match_operand:SHORT 1 "riscv_atomic_memory_operand" "+A")) (set (match_dup 1) (unspec_volatile:SHORT [(any_atomic:SHORT (match_dup 1) @@ -331,7 +331,7 @@ (define_expand "lrsc_atomic_fetch_" [(match_operand:SHORT 0 "register_operand") ;; old value at mem - (any_atomic:SHORT (match_operand:SHORT 1 "memory_operand") ;; mem location + (any_atomic:SHORT (match_operand:SHORT 1 "riscv_atomic_memory_operand") ;; mem location (match_operand:SHORT 2 "reg_or_0_operand")) ;; value for op (match_operand:SI 3 "const_int_operand")] ;; model "!TARGET_ZABHA && TARGET_ZALRSC && TARGET_INLINE_SUBWORD_ATOMIC" @@ -374,7 +374,7 @@ (define_expand "atomic_exchange" [(match_operand:GPR 0 "register_operand") ;; old value at mem - (match_operand:GPR 1 "memory_operand") ;; mem location + (match_operand:GPR 1 "riscv_atomic_memory_operand") ;; mem location (match_operand:GPR 2 "register_operand") ;; value for op (match_operand:SI 3 "const_int_operand")] ;; model "TARGET_ZAAMO || TARGET_ZALRSC" @@ -403,7 +403,7 @@ (define_insn "amo_atomic_exchange" [(set (match_operand:GPR 0 "register_operand" "=r") (unspec_volatile:GPR - [(match_operand:GPR 1 "memory_operand" "+A") + [(match_operand:GPR 1 "riscv_atomic_memory_operand" "+A") (match_operand:SI 3 "const_int_operand")] ;; model UNSPECV_SYNC_EXCHANGE)) (set (match_dup 1) @@ -416,7 +416,7 @@ (define_insn "amo_atomic_exchange_extended" [(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_atomic_memory_operand" "+A") (match_operand:SI 3 "const_int_operand")] ;; model UNSPECV_SYNC_EXCHANGE))) (set (match_dup 1) @@ -429,7 +429,7 @@ (define_insn "lrsc_atomic_exchange" [(set (match_operand:GPR 0 "register_operand" "=&r") (unspec_volatile:GPR - [(match_operand:GPR 1 "memory_operand" "+A") + [(match_operand:GPR 1 "riscv_atomic_memory_operand" "+A") (match_operand:SI 3 "const_int_operand")] ;; model UNSPECV_SYNC_EXCHANGE)) (set (match_dup 1) @@ -447,7 +447,7 @@ (define_expand "atomic_exchange" [(match_operand:SHORT 0 "register_operand") ;; old value at mem - (match_operand:SHORT 1 "memory_operand") ;; mem location + (match_operand:SHORT 1 "riscv_atomic_memory_operand") ;; mem location (match_operand:SHORT 2 "register_operand") ;; value (match_operand:SI 3 "const_int_operand")] ;; model "(TARGET_ZALRSC && TARGET_INLINE_SUBWORD_ATOMIC) || TARGET_ZABHA" @@ -464,7 +464,7 @@ (define_insn "zabha_atomic_exchange" [(set (match_operand:SHORT 0 "register_operand" "=r") (unspec_volatile:SHORT - [(match_operand:SHORT 1 "memory_operand" "+A") + [(match_operand:SHORT 1 "riscv_atomic_memory_operand" "+A") (match_operand:SI 3 "const_int_operand")] ;; model UNSPECV_SYNC_EXCHANGE_ZABHA)) (set (match_dup 1) @@ -476,7 +476,7 @@ (define_expand "lrsc_atomic_exchange" [(match_operand:SHORT 0 "register_operand") ;; old value at mem - (match_operand:SHORT 1 "memory_operand") ;; mem location + (match_operand:SHORT 1 "riscv_atomic_memory_operand") ;; mem location (match_operand:SHORT 2 "register_operand") ;; value (match_operand:SI 3 "const_int_operand")] ;; model "!TARGET_ZABHA && TARGET_ZALRSC && TARGET_INLINE_SUBWORD_ATOMIC" @@ -509,7 +509,7 @@ (define_insn "subword_atomic_exchange_strong" [(set (match_operand:SI 0 "register_operand" "=&r") ;; old value at mem - (match_operand:SI 1 "memory_operand" "+A")) ;; mem location + (match_operand:SI 1 "riscv_atomic_memory_operand" "+A")) ;; mem location (set (match_dup 1) (unspec_volatile:SI [(match_operand:SI 2 "arith_operand" "rI") ;; value @@ -538,7 +538,7 @@ ;; More details: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/444 (define_insn "zacas_atomic_cas_value_strong" [(set (match_operand:GPR 0 "register_operand" "=&r") ;; val output - (match_operand:GPR 1 "memory_operand" "+A")) ;; memory + (match_operand:GPR 1 "riscv_atomic_memory_operand" "+A")) ;; memory (set (match_dup 1) (unspec_volatile:GPR [(match_operand:GPR 2 "register_operand" "0") ;; expected val (match_operand:GPR 3 "reg_or_0_operand" "rJ") ;; desired val @@ -566,7 +566,7 @@ (define_insn "zalrsc_atomic_cas_value_strong" [(set (match_operand:GPR 0 "register_operand" "=&r") - (match_operand:GPR 1 "memory_operand" "+A")) + (match_operand:GPR 1 "riscv_atomic_memory_operand" "+A")) (set (match_dup 1) (unspec_volatile:GPR [(match_operand:GPR 2 "reg_or_0_operand" "rJ") (match_operand:GPR 3 "reg_or_0_operand" "rJ") @@ -594,7 +594,7 @@ (define_expand "atomic_compare_and_swap" [(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 2 "riscv_atomic_memory_operand" "") ;; memory (match_operand:GPR 3 "register_operand" "") ;; expected value (match_operand:GPR 4 "reg_or_0_operand" "") ;; desired value (match_operand:SI 5 "const_int_operand" "") ;; is_weak @@ -651,7 +651,7 @@ ;; More details: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/444 (define_insn "zacas_atomic_cas_value_strong" [(set (match_operand:SHORT 0 "register_operand" "=&r") ;; val output - (match_operand:SHORT 1 "memory_operand" "+A")) ;; memory + (match_operand:SHORT 1 "riscv_atomic_memory_operand" "+A")) ;; memory (set (match_dup 1) (unspec_volatile:SHORT [(match_operand:SHORT 2 "register_operand" "0") ;; expected_val (match_operand:SHORT 3 "reg_or_0_operand" "rJ") ;; desired_val @@ -680,7 +680,7 @@ (define_expand "atomic_compare_and_swap" [(match_operand:SI 0 "register_operand") ;; bool output (match_operand:SHORT 1 "register_operand") ;; val output - (match_operand:SHORT 2 "memory_operand") ;; memory + (match_operand:SHORT 2 "riscv_atomic_memory_operand") ;; memory (match_operand:SHORT 3 "register_operand") ;; expected value (match_operand:SHORT 4 "reg_or_0_operand") ;; desired value (match_operand:SI 5 "const_int_operand") ;; is_weak @@ -728,7 +728,7 @@ (define_expand "zalrsc_atomic_cas_value_strong" [(match_operand:SHORT 0 "register_operand") ;; val output - (match_operand:SHORT 1 "memory_operand") ;; memory + (match_operand:SHORT 1 "riscv_atomic_memory_operand") ;; memory (match_operand:SHORT 2 "reg_or_0_operand") ;; expected value (match_operand:SHORT 3 "reg_or_0_operand") ;; desired value (match_operand:SI 4 "const_int_operand") ;; mod_s @@ -783,7 +783,7 @@ (define_insn "subword_atomic_cas_strong" [(set (match_operand:SI 0 "register_operand" "=&r") ;; old value at mem - (match_operand:SI 1 "memory_operand" "+A")) ;; mem location + (match_operand:SI 1 "riscv_atomic_memory_operand" "+A")) ;; mem location (set (match_dup 1) (unspec_volatile:SI [(match_operand:SI 2 "reg_or_0_operand" "rJ") ;; expected value (match_operand:SI 3 "arith_operand" "rI")] ;; desired value @@ -809,7 +809,7 @@ (define_expand "atomic_test_and_set" [(match_operand:QI 0 "register_operand" "") ;; bool output - (match_operand:QI 1 "memory_operand" "+A") ;; memory + (match_operand:QI 1 "riscv_atomic_memory_operand" "+A") ;; memory (match_operand:SI 2 "const_int_operand" "")] ;; model "TARGET_ZAAMO || TARGET_ZALRSC" { diff --git a/gcc/testsuite/gcc.target/riscv/pr108031.c b/gcc/testsuite/gcc.target/riscv/pr108031.c new file mode 100644 index 00000000000..6f7e13a4b2b --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/pr108031.c @@ -0,0 +1,31 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -march=rv64gc -mabi=lp64d -fdump-rtl-expand -fdump-rtl-cse1" { target { rv64 } } } */ +/* { dg-options "-O2 -march=rv32gc -mabi=ilp32d -fdump-rtl-expand -fdump-rtl-cse1" { target { rv32 } } } */ + +struct s { + int a; + int b; +}; + +struct s s; + +int f(void) +{ + return __atomic_fetch_add(&s.a, 1, 0) + __atomic_fetch_add(&s.b, 1, 0); +} + + +struct s2 { + long a; + long b; +}; + +struct s2 s2; + +long f2(void) +{ + return __atomic_fetch_add(&s2.a, 1, 0) + __atomic_fetch_add(&s2.b, 1, 0); +} + +/* { dg-final { scan-rtl-dump-not "mem\[^\r\n]*lo_sum" "expand" } } */ +/* { dg-final { scan-rtl-dump-not "mem\[^\r\n]*lo_sum" "cse1" } } */