Re: [PATCH AArch64]Fix test failure for pr84682-2.c

[Joey Ye <joey.ye.cc@gmail.com>]

Hi Bin & Richard,

It is not as simple as keeping the assertion, which still fails even
with the change in reorg.c. The testing result is as following:

I. With Bin's patch version 2 (removing the assertion in aarch64.c and
adding the check in reorg.c): pr84682-2.c passes

II. With Richard's suggestion to keep the assertion in aarch64, but
adding the check in reorg.c: pr84682-2.c fails

Apparently there is a different path that reaches the assertion.

With II:
/build/trunk/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c: In
function 'b': /build/trunk/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c:10:1:
internal compiler error: in aarch64_classify_address, at
config/aarch64/aarch64.c:5721
0xfa4071 aarch64_classify_address
        /build/trunk/src/gcc/gcc/config/aarch64/aarch64.c:5720
0xfa94f3 aarch64_legitimate_address_hook_p
        /build/trunk/src/gcc/gcc/config/aarch64/aarch64.c:6003
0xb85661 strict_memory_address_addr_space_p(machine_mode, rtx_def*,
unsigned char)
        /build/trunk/src/gcc/gcc/reload.c:2177
0xb75da9 constrain_operands(int, unsigned long)
        /build/trunk/src/gcc/gcc/recog.c:2706
0xb761a0 extract_constrain_insn(rtx_insn*)
        /build/trunk/src/gcc/gcc/recog.c:2210
0xa6dd57 check_rtl
        /build/trunk/src/gcc/gcc/lra.c:2182
0xa737db lra(_IO_FILE*)
        /build/trunk/src/gcc/gcc/lra.c:2616
0xa25989 do_reload
        /build/trunk/src/gcc/gcc/ira.c:5469
0xa25989 execute

Current trunk without any patch:
/build/trunk/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c: In
function 'b': /build/trunk/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c:9:3:
internal compiler error: in aarch64_classify_address, at
config/aarch64/aarch64.c:5721
0xfa4011 aarch64_classify_address
        /build/trunk/src/gcc/gcc/config/aarch64/aarch64.c:5720
0xfa9493 aarch64_legitimate_address_hook_p
        /build/trunk/src/gcc/gcc/config/aarch64/aarch64.c:6003
0xb74cf3 memory_address_addr_space_p(machine_mode, rtx_def*, unsigned char)
        /build/trunk/src/gcc/gcc/recog.c:1334
0xb74cf3 address_operand(rtx_def*, machine_mode)
        /build/trunk/src/gcc/gcc/recog.c:1073
0xb74cf3 asm_operand_ok(rtx_def*, char const*, char const**)
        /build/trunk/src/gcc/gcc/recog.c:1817
0x75e591 expand_asm_stmt
        /build/trunk/src/gcc/gcc/cfgexpand.c:3135
0x766d67 expand_gimple_stmt_1
        /build/trunk/src/gcc/gcc/cfgexpand.c:3572
0x766d67 expand_gimple_stmt
        /build/trunk/src/gcc/gcc/cfgexpand.c:3734
0x768ce7 expand_gimple_basic_block

More places need to be patched.

Thanks,
Joey
On Thu, Aug 30, 2018 at 2:02 AM Bin.Cheng <amker.cheng@gmail.com> wrote:
>
> On Thu, Aug 30, 2018 at 2:47 AM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
> >
> > Joey Ye <joey.ye.cc@gmail.com> writes:
> > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> > > index 07c55b1..9e965ab 100644
> > > --- a/gcc/config/aarch64/aarch64.c
> > > +++ b/gcc/config/aarch64/aarch64.c
> > > @@ -5674,9 +5674,6 @@ aarch64_classify_address (struct aarch64_address_info *info,
> > >        && (code != POST_INC && code != REG))
> > >      return false;
> > >
> > > -  gcc_checking_assert (GET_MODE (x) == VOIDmode
> > > -                    || SCALAR_INT_MODE_P (GET_MODE (x)));
> > > -
> > >    switch (code)
> > >      {
> > >      case REG:
> > > diff --git a/gcc/recog.c b/gcc/recog.c
> > > index 0a8fa2c..510aba2 100644
> > > --- a/gcc/recog.c
> > > +++ b/gcc/recog.c
> > > @@ -1070,6 +1070,11 @@ general_operand (rtx op, machine_mode mode)
> > >  int
> > >  address_operand (rtx op, machine_mode mode)
> > >  {
> > > +  /* Wrong mode for an address expr.  */
> > > +  if (GET_MODE (op) != VOIDmode
> > > +      && ! SCALAR_INT_MODE_P (GET_MODE (op)))
> > > +    return false;
> > > +
> > >    return memory_address_p (mode, op);
> > >  }
> > >
> >
> > The address_operand part is OK, thanks.
> >
> > I think we should keep the assert in aarch64_classify_address, since
> > IMO it's a bug for anything else to reach that point.
>
> Hi Joey,
> Could you help me update the patch as suggested by Richard and commit
> it please?  My new assignment is still on the way.
> Thanks very much!
>
> Thanks,
> bin
> >
> > Richard