On Wed, Jul 24, 2024 at 3:57 PM Jeff Law <jeffreya...@gmail.com> wrote:
>
>
>
> On 7/24/24 7:31 AM, Christoph Müllner wrote:
> > When enabling XTheadMemIdx, we enable the pre- and post-modify
> > addressing modes in the RISC-V backend.
> > Unfortunately, the auto_inc_dec pass will then attempt to utilize
> > this feature regardless of the mode class (i.e. scalar or vector).
> > The assumption seems to be, that an enabled addressing mode for
> > scalar instructions will also be available for vector instructions.
> >
> > In case of XTheadMemIdx and RVV, this is ovbiously not the case.
> > Still, auto_inc_dec (-O3) performs optimizations like the following:
> >
> > (insn 23 20 27 3 (set (mem:V4QI (reg:DI 136 [ ivtmp.13 ]) [0 MEM <vector(4)
> > char> [(char *)_39]+0 S4 A32])
> > (reg:V4QI 168)) "gcc/testsuite/gcc.target/riscv/pr116033.c":12:27
> > 3183 {*movv4qi}
> > (nil))
> > (insn 40 39 41 3 (set (reg:DI 136 [ ivtmp.13 ])
> > (plus:DI (reg:DI 136 [ ivtmp.13 ])
> > (const_int 20 [0x14]))) 5 {adddi3}
> > (nil))
> > ====>
> > (insn 23 20 27 3 (set (mem:V4QI (post_modify:DI (reg:DI 136 [ ivtmp.13 ])
> > (plus:DI (reg:DI 136 [ ivtmp.13 ])
> > (const_int 20 [0x14]))) [0 MEM <vector(4) char> [(char
> > *)_39]+0 S4 A32])
> > (reg:V4QI 168)) "gcc/testsuite/gcc.target/riscv/pr116033.c":12:27
> > 3183 {*movv4qi}
> > (expr_list:REG_INC (reg:DI 136 [ ivtmp.13 ])
> > (nil)))
> >
> > The resulting memory-store with post-modify addressing cannot be
> > lowered to an existing instruction (see PR116033).
> > At a lower optimization level (-O2) this is currently fine,
> > but we can't rely on that.
> >
> > One solution would be to introduce a target hook to check if a certain
> > type can be used for pre-/post-modify optimizations.
> > However, it will be hard to justify such a hook, if only a single
> > RISC-V vendor extension requires that.
> > Therefore, this patch takes a more drastic approach and disables
> > pre-/post-modify addressing if TARGET_VECTOR is set.
> > This results in not emitting pre-/post-modify instructions from
> > XTheadMemIdx if RVV is enabled.
> >
> > Note, that this is not an issue with XTheadVector, because
> > we currently don't have auto-vectorization for that extension.
> > To ensure this won't changed without being noticed, an additional
> > test is added.
> >
> > PR target/116033
> >
> > gcc/ChangeLog:
> >
> > * config/riscv/riscv.h (HAVE_POST_MODIFY_DISP): Disable for RVV.
> > (HAVE_PRE_MODIFY_DISP): Likewise.
> This sounds like it's just papering over the real bug, probably in the
> address checking code of the backend.
>
> I haven't tried to debug this, but I'd look closely at
> th_classify_address which seems to ignore the mode, which seems wrong.
I checked that, and there is a mode check there.
But, after your comment, I challenged the test and indeed:
if (!(INTEGRAL_MODE_P (mode) && GET_MODE_SIZE (mode).to_constant ()
<= 8)) return false;
INTEGRAL_MODE_P() includes vector modes.
I'll change the check to ensure that "GET_MODE_CLASS (MODE) ==
MODE_INT" is fulfilled and prepare a v2.
Thank you!
