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

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

typo: s/reorg.c/recog.c/g
On Thu, Aug 30, 2018 at 11:20 AM Joey Ye <joey.ye.cc@gmail.com> wrote:
>
> 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