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.
Jeff
PR target/108031
gcc
* config/riscv/predicates.md (riscv_atomic_memory_operand): New
predicate.
* config/riscv/sync.md (<atomic_optab><mode> pattern and expander):
Use riscv_atomic_memory_operand.
(amo_atomic_<atomic_optab><mode>): Likewise.
(lrsc_atomic_<atomic_optab><mode>): Likewise.
(atomic_fetch_<atomic_optab><mode): Likewise.
(amo_atomic_fetch_<atomic_optab><mode>): Likewise.
(lrsc_atomic_fetch_<atomic_optab><mode>): Likewise.
(subword_atomic_fetch_strong_<atomic_optab>): Likewise.
(atomic_fetch_nand<mode>): Likewise.
(subword_atomic_fetch_strong_nand): Likewise.
(zabha_atomic_fetch_<atomic_optab><mode>): Likewise.
(lrsc_atomic_fetch_<atomic_optab><mode>): Likewise.
(atomic_exchange<mode>): Likewise.
(amo_atomic_exchange<mode>): Likewise.
(amo_atomic_exchange<mode>_extended): Likewise.
(lrsc_atomic_exchange<mode>): Likewise.
(subword_atomic_exchange_strong): Likewise.
(zacas_atomic_cas_value_strong<mode>): Likewise.
(zalrsc_atomic_cas_value_strong<mode>): 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_si"
(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 @@ (define_expand "atomic_store<mode>"
;; AMO ops
(define_insn "atomic_<atomic_optab><mode>"
- [(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 @@ (define_insn "atomic_<atomic_optab><mode>"
(set (attr "length") (const_int 4))])
(define_expand "atomic_<atomic_optab><mode>"
- [(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_expand "atomic_<atomic_optab><mode>"
})
(define_insn "amo_atomic_<atomic_optab><mode>"
- [(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 @@ (define_insn "amo_atomic_<atomic_optab><mode>"
(set (attr "length") (const_int 4))])
(define_insn "lrsc_atomic_<atomic_optab><mode>"
- [(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_insn "lrsc_atomic_<atomic_optab><mode>"
(define_expand "atomic_fetch_<atomic_optab><mode>"
[(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_expand "atomic_fetch_<atomic_optab><mode>"
(define_insn "amo_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_atomic_memory_operand" "+A"))
(set (match_dup 1)
(unspec_volatile:GPR
[(any_atomic:GPR (match_dup 1)
@@ -185,7 +185,7 @@ (define_insn "amo_atomic_fetch_<atomic_optab><mode>"
(define_insn "lrsc_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_atomic_memory_operand" "+A"))
(set (match_dup 1)
(unspec_volatile:GPR
[(any_atomic:GPR (match_dup 1)
@@ -206,7 +206,7 @@ (define_insn "lrsc_atomic_fetch_<atomic_optab><mode>"
(define_insn "subword_atomic_fetch_strong_<atomic_optab>"
[(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_insn "subword_atomic_fetch_strong_<atomic_optab>"
(define_expand "atomic_fetch_nand<mode>"
[(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_expand "atomic_fetch_nand<mode>"
(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_insn "subword_atomic_fetch_strong_nand"
(define_expand "atomic_fetch_<atomic_optab><mode>"
[(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_expand "atomic_fetch_<atomic_optab><mode>"
(define_insn "zabha_atomic_fetch_<atomic_optab><mode>"
[(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_insn "zabha_atomic_fetch_<atomic_optab><mode>"
(define_expand "lrsc_atomic_fetch_<atomic_optab><mode>"
[(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 "lrsc_atomic_fetch_<atomic_optab><mode>"
(define_expand "atomic_exchange<mode>"
[(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_expand "atomic_exchange<mode>"
(define_insn "amo_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_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<mode>"
(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 "amo_atomic_exchange_extended"
(define_insn "lrsc_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_atomic_memory_operand" "+A")
(match_operand:SI 3 "const_int_operand")] ;; model
UNSPECV_SYNC_EXCHANGE))
(set (match_dup 1)
@@ -447,7 +447,7 @@ (define_insn "lrsc_atomic_exchange<mode>"
(define_expand "atomic_exchange<mode>"
[(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_expand "atomic_exchange<mode>"
(define_insn "zabha_atomic_exchange<mode>"
[(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_insn "zabha_atomic_exchange<mode>"
(define_expand "lrsc_atomic_exchange<mode>"
[(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_expand "lrsc_atomic_exchange<mode>"
(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 @@ (define_insn "subword_atomic_exchange_strong"
;; More details:
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/444
(define_insn "zacas_atomic_cas_value_strong<mode>"
[(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 "zacas_atomic_cas_value_strong<mode>"
(define_insn "zalrsc_atomic_cas_value_strong<mode>"
[(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_insn "zalrsc_atomic_cas_value_strong<mode>"
(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 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 @@ (define_expand "atomic_compare_and_swap<mode>"
;; More details:
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/444
(define_insn "zacas_atomic_cas_value_strong<mode>"
[(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_insn "zacas_atomic_cas_value_strong<mode>"
(define_expand "atomic_compare_and_swap<mode>"
[(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 "atomic_compare_and_swap<mode>"
(define_expand "zalrsc_atomic_cas_value_strong<mode>"
[(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_expand "zalrsc_atomic_cas_value_strong<mode>"
(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_insn "subword_atomic_cas_strong"
(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" } } */
