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
