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



Reply via email to