Re: [PATCH][ARM/AArch64] PR 68088: Fix RTL checking ICE due to subregs inside accumulator forwarding check

[Kyrill Tkachov <kyrylo dot tkachov at arm dot com>]

Hi Marcus,

On 29/10/15 13:46, Marcus Shawcroft wrote:
> On 28 October 2015 at 10:07, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
> > Hi all,
> >
> > This RTL checking error occurs on aarch64 in aarch_accumulator_forwarding
> > when processing an msubsi insn
> > with subregs:
> > (insn 15 14 16 3 (set (reg/v:SI 78 [ i ])
> >          (minus:SI (subreg:SI (reg/v:DI 76 [ aul ]) 0)
> >              (mult:SI (subreg:SI (reg:DI 83) 0)
> >                  (subreg:SI (reg:DI 75 [ _20 ]) 0)))) schedice.c:10 357
> > {*msubsi}
> >
> > The register_operand predicate for that pattern allows subregs (I think
> > correctly).
> > The code in aarch_accumulator_forwarding doesn't take that into account and
> > ends up
> > taking a REGNO of a SUBREG, causing a checking error.
> >
> > This patch fixes that by stripping the subregs off the accumulator rtx
> > before
> > checking that the inner expression is a REG and taking its REGNO.
> >
> > The testcase now works fine with an aarch64-none-elf toolchain configure for
> > RTL checking.
> >
> > The testcase is taken verbatim from the BZ entry for PR 68088.
> > Since this function is shared between arm and aarch64 I've bootstrapped and
> > tested it on both
> > and I'll need ok's for both ports.
> >
> > Ok for trunk?
> rtl.h exposes reg_or_subregno() already doesn't that do what we need here?

reg_or_subregno assumes that what it's passed is REG or a SUBREG.
It will ICE on any other rtx. Here I want to strip the subreg if it is
a subreg, but leave it as it is otherwise.

> The test case is not aarch64 specific therefore I think convention is
> that it should go into a generic directory.

Ok, I'll put it in gcc.dg/

Thanks,
Kyrill

> Cheers
> /Marcus