On 10/7/26 7:21 AM, [email protected] wrote:
From: Kyrylo Tkachov <[email protected]>
cselib looks up memory values only in the mode of the access. A value
that is stored or loaded in one mode and then loaded in another mode of
the same size therefore misses a register that already holds it. For
example, SVE spill code can contain:
(set (mem:VNx4SI (plus:DI (reg:DI sp) (const_int K))) (reg:VNx4SI v7))
...
(set (reg:VNx2DI v23) (mem:VNx2DI (plus:DI (reg:DI sp) (const_int K))))
where the load can become:
(set (reg:VNx2DI v23) (reg:VNx2DI v7))
Scalar integer and floating-point modes have the same problem. DSE
forwards stored values across mode changes, but only before register
allocation, because it needs a new pseudo.
Add cselib_lookup_mem_other_modes, which returns the values recorded at
the address of a MEM in other modes of the same size. Split the filters
and the address lookup out of cselib_lookup_mem, so that both functions
use them. Like cselib_lookup_mem, ignore floating-point modes under
-ffloat-store.
In reload_cse_simplify_set, view the hard registers that hold these
values in the mode of the load with simplify_subreg, which checks
REG_CAN_CHANGE_MODE_P. Add them to the existing candidates, so that the
existing cost comparison, LOAD_EXTEND_OP handling and instruction
validation apply. As in LRA and regcprop, this relies on
REG_CAN_CHANGE_MODE_P to allow a mode change only if the register then
holds the bytes that memory would hold in the new mode.
The motivating code is a dav1d loop filter compiled
with -march=armv9-a+sve2 -msve-vector-bits=256.
A testcase derived from it is given in
gcc.target/aarch64/sve/spill-mode-change.c.
At -O3 this patch removes 12% of its SVE vector loads (ldr z), and 8% with
-fno-ivopts.
All the removed loads are reloads of spill slots. The instruction count
of the aarch64 cc1 for -O2 compiles of combine.ii, dwarf2out.ii,
fold-const.ii and sqlite3.i changes by -0.01% to +0.01%.
Bootstrapped and tested on aarch64-none-linux-gnu and x86_64-linux.
Ok for trunk?
Thanks,
Kyrill
Cc: Jeff Law <[email protected]>
gcc/ChangeLog:
* cselib.cc (cselib_lookup_mem_addr): New function, split out of...
(cselib_lookup_mem): ...here.
(cselib_lookup_mem_other_modes): New function.
* cselib.h (cselib_lookup_mem_other_modes): Declare.
* postreload.cc (reload_cse_add_mem_regs): New function.
(reload_cse_simplify_set): Also consider registers that hold the
loaded value in another mode of the same size.
gcc/testsuite/ChangeLog:
* gcc.dg/rtl/aarch64/postreload-mode-forward.c: New test.
* gcc.dg/rtl/aarch64/postreload-mode-forward-be.c: Likewise.
* gcc.dg/rtl/aarch64/postreload-mode-forward-call.c: Likewise.
* gcc.dg/rtl/aarch64/postreload-mode-forward-extend.c: Likewise.
* gcc.dg/rtl/aarch64/postreload-mode-forward-float-store.c: Likewise.
* gcc.target/aarch64/sve/spill-mode-change.c: Likewise.
* gcc.target/i386/addr-space-10.c: Likewise.
Signed-off-by: Kyrylo Tkachov <[email protected]>
---
Changes since v1:
- Handle all modes, not only vector modes (Jeff).
- Add a cselib query instead of exporting elt_list and copying the
address lookup of cselib_lookup_mem.
- Add the candidates to the existing loop in reload_cse_simplify_set,
so that the cost check and the LOAD_EXTEND_OP handling apply.
- Apply -ffloat-store to the stored mode too.
- More tests: scalar modes, calls, -ffloat-store, big-endian, a mode
change that the target rejects, x86 address spaces.
gcc/cselib.cc | 64 ++++-
gcc/cselib.h | 1 +
gcc/postreload.cc | 34 ++-
.../rtl/aarch64/postreload-mode-forward-be.c | 29 +++
.../aarch64/postreload-mode-forward-call.c | 70 ++++++
.../aarch64/postreload-mode-forward-extend.c | 62 +++++
.../postreload-mode-forward-float-store.c | 102 ++++++++
.../rtl/aarch64/postreload-mode-forward.c | 228 ++++++++++++++++++
.../aarch64/sve/spill-mode-change.c | 95 ++++++++
gcc/testsuite/gcc.target/i386/addr-space-10.c | 39 +++
10 files changed, 708 insertions(+), 16 deletions(-)
create mode 100644
gcc/testsuite/gcc.dg/rtl/aarch64/postreload-mode-forward-be.c
create mode 100644
gcc/testsuite/gcc.dg/rtl/aarch64/postreload-mode-forward-call.c
create mode 100644
gcc/testsuite/gcc.dg/rtl/aarch64/postreload-mode-forward-extend.c
create mode 100644
gcc/testsuite/gcc.dg/rtl/aarch64/postreload-mode-forward-float-store.c
create mode 100644 gcc/testsuite/gcc.dg/rtl/aarch64/postreload-mode-forward.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/spill-mode-change.c
create mode 100644 gcc/testsuite/gcc.target/i386/addr-space-10.c
diff --git a/gcc/cselib.cc b/gcc/cselib.cc
index 5567caa47..020fecb2d 100644
--- a/gcc/cselib.cc
+++ b/gcc/cselib.cc
@@ -1765,6 +1765,30 @@ add_mem_for_addr (cselib_val *addr_elt, cselib_val
*mem_elt, rtx x)
}
}
+/* Return the canonical value for the address of X, which is a MEM rtx.
+ If CREATE, make a new value for the address if we haven't seen it
+ before. Return null if cselib does not record X or if the address
+ has no value. */
+
+static cselib_val *
+cselib_lookup_mem_addr (rtx x, int create)
+{
+ machine_mode mode = GET_MODE (x);
+
+ if (MEM_VOLATILE_P (x) || mode == BLKmode
+ || !cselib_record_memory
+ || (FLOAT_MODE_P (mode) && flag_float_store))
+ return 0;
+
+ machine_mode addr_mode = GET_MODE (XEXP (x, 0));
+ if (addr_mode == VOIDmode)
+ addr_mode = Pmode;
I guess we can get VOIDmode as the address might be a CONST_INT or
similar kind of modeless expression? Given you're just copying that
test, let's just go with it as-is.
Given this is returning a pointer, returning NULL seems better than
returning "0". A nit, but that's generally what we try to do.
OK with the return 0 turned into RETURN_NULL and prereqs installed.
jeff
