[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 (<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 df1a76049f40d5fe02acf62ac14f4afc5dbb5339..3ee9042b949c3557ecf2f921105bce30a98753b6 100644 (file)
--- 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 a046519c8351501e7e6157a2e3c6792131baa705..0d983ce6024573427319841e19bd35ef0f71392c 100644 (file)
--- a/gcc/config/riscv/sync.md
+++ b/gcc/config/riscv/sync.md
@@ -94,7 +94,7 @@
 ;; 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 @@
    (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_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 @@
    (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_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_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 "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 "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_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_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_<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_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_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 "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_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_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<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_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_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_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_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<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 "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_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 @@
 ;; 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_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 "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_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 (file)
index 0000000..6f7e13a
--- /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" } } */