| Message ID | 20240730161707.853999-1-christoph.muellner@vrull.eu |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: xtheadmemidx: Fix RV32 ICE because of unexpected subreg | expand |
On 7/30/24 10:17 AM, Christoph Müllner wrote: > As documented in PR116131, we might end up with the following > INSN for rv32i_xtheadmemidx after th_memidx_I_c is applied: > > (insn 18 14 0 2 (set (mem:SI (plus:SI (reg/f:SI 141) > (ashift:SI (subreg:SI (reg:DI 134 [ a.0_1 ]) 0) > (const_int 2 [0x2]))) [0 S4 A32]) > (reg:SI 143 [ b ])) "<source>":4:17 -1 > (nil)) > > This INSN is rejected by th_memidx_classify_address_index(), > because the first ASHIFT operand needs to satisfy REG_P(). > > For most other cases of subreg expressions, an extension/trunctation > will be created before the ASHIFT INSN. However, this case is different > as we have a reg:DI and RV32, where no truncation is possible. > Therefore, this patch accepts this corner-case and allows this INSN. > > PR target/116131 > > gcc/ChangeLog: > > * config/riscv/thead.cc (th_memidx_classify_address_index): > Allow (ashift (subreg:SI (reg:DI))) for RV32. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/pr116131.c: New test. But why do we have the ashift form here at all? Canonicalization rules say this is invalid RTL. So while this patch fixes the ICE it does not address the canonicalization problem in this RTL. Specifically in a memory address we should be using mult rather than ashift. And for associative ops, we chain left. So the proper form of the address (inside a MEM) is: (plus (mult (op1) (const_int 4) (op2)) That needs to be fixed. jeff
diff --git a/gcc/config/riscv/thead.cc b/gcc/config/riscv/thead.cc index 6f5edeb7e0a..6cbbece3ec0 100644 --- a/gcc/config/riscv/thead.cc +++ b/gcc/config/riscv/thead.cc @@ -646,6 +646,19 @@ th_memidx_classify_address_index (struct riscv_address_info *info, rtx x, shift = INTVAL (XEXP (offset, 1)); offset = XEXP (offset, 0); } + /* (ashift:SI (subreg:SI (reg:DI)) (const_int shift)) */ + else if (GET_CODE (offset) == ASHIFT + && GET_MODE (offset) == SImode + && SUBREG_P (XEXP (offset, 0)) + && GET_MODE (XEXP (offset, 0)) == SImode + && GET_MODE (XEXP (XEXP (offset, 0), 0)) == DImode + && CONST_INT_P (XEXP (offset, 1)) + && IN_RANGE (INTVAL (XEXP (offset, 1)), 0, 3)) + { + type = ADDRESS_REG_REG; + shift = INTVAL (XEXP (offset, 1)); + offset = XEXP (XEXP (offset, 0), 0); + } /* (ashift:DI (zero_extend:DI (reg:SI)) (const_int shift)) */ else if (GET_CODE (offset) == ASHIFT && GET_MODE (offset) == DImode diff --git a/gcc/testsuite/gcc.target/riscv/pr116131.c b/gcc/testsuite/gcc.target/riscv/pr116131.c new file mode 100644 index 00000000000..4d644c37cde --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/pr116131.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-skip-if "" { *-*-* } { "-flto" "-O0" "-Og" "-Os" "-Oz" } } */ +/* { dg-options "-march=rv64gc_xtheadmemidx" { target { rv64 } } } */ +/* { dg-options "-march=rv32gc_xtheadmemidx" { target { rv32 } } } */ + +volatile long long a; +int b; +int c[1]; + +void d() +{ + c[a] = b; +} + +/* { dg-final { scan-assembler "th.srw\t" } } */
As documented in PR116131, we might end up with the following INSN for rv32i_xtheadmemidx after th_memidx_I_c is applied: (insn 18 14 0 2 (set (mem:SI (plus:SI (reg/f:SI 141) (ashift:SI (subreg:SI (reg:DI 134 [ a.0_1 ]) 0) (const_int 2 [0x2]))) [0 S4 A32]) (reg:SI 143 [ b ])) "<source>":4:17 -1 (nil)) This INSN is rejected by th_memidx_classify_address_index(), because the first ASHIFT operand needs to satisfy REG_P(). For most other cases of subreg expressions, an extension/trunctation will be created before the ASHIFT INSN. However, this case is different as we have a reg:DI and RV32, where no truncation is possible. Therefore, this patch accepts this corner-case and allows this INSN. PR target/116131 gcc/ChangeLog: * config/riscv/thead.cc (th_memidx_classify_address_index): Allow (ashift (subreg:SI (reg:DI))) for RV32. gcc/testsuite/ChangeLog: * gcc.target/riscv/pr116131.c: New test. Reported-by: Patrick O'Neill <patrick@rivosinc.com> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu> --- gcc/config/riscv/thead.cc | 13 +++++++++++++ gcc/testsuite/gcc.target/riscv/pr116131.c | 15 +++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 gcc/testsuite/gcc.target/riscv/pr116131.c