[PATCH 2/2 v3][IRA,LRA] Fix PR86939, IRA incorrectly creates an interference between a pseudo register and a hard register

[Richard Biener]

On Thu, Nov 8, 2018 at 11:57 AM Renlin Li <renlin.li@foss.arm.com> wrote:
>
> Hi,
>
> On 11/06/2018 06:58 PM, Jeff Law wrote:
> > On 11/6/18 3:52 AM, Renlin Li wrote:
> >> Hi Jeff & Peter,
> >>
> >> On 11/05/2018 07:41 PM, Jeff Law wrote:
> >>> On 11/5/18 12:36 PM, Peter Bergner wrote:
> >>>> On 11/5/18 1:20 PM, Jeff Law wrote:
> >>>>> On 11/1/18 4:07 PM, Peter Bergner wrote:
> >>>>>> On 11/1/18 1:50 PM, Renlin Li wrote:
> >>>>>>> Is there any update on this issues?
> >>>>>>> arm-none-linux-gnueabihf native toolchain has been mis-compiled
> >>>>>>> for a while.
> >>>>>>
> >>>>>>   From the analysis I've done, my commit is just exposing latent issues
> >>>>>> in LRA.  Can you try the patch I submitted here to see if it helps?
> >>>>>>
> >>>>>>     https://gcc.gnu.org/ml/gcc-patches/2018-10/msg01757.html
> >>>>>>
> >>>>>> It survives on powerpc64le-linux, x86_64-linux and s390x-linux.
> >>>>>> Jeff threw it on his testers and said he saw an arm issue and was
> >>>>>> trying to come up with a test case for me to debug.
> >>>>> So I don't think the ARM issues are related to your patch, they may
> >>>>> have
> >>>>> been related the combiner changes that went in around the same time.
> >> Yes, there are issues related to the combiner changes.
> >>
> >> But the IRA/LRA change dose cause the arm-none-linux-gnueabihf bootstrap
> >> native toolchain mis-compiled.
> >> And the new patch seems not fix this problem.
> > That's strange.  I'm bootstrapping arm-linux-gnueabihf daily with qemu +
> > a suitable root filesystem using Peter's most recent testing patch.
> >
> >
> >>
> >> I am trying to extract a test case, but it is a little bit hard as the
> >> toolchain itself is mis-compiled.
> >> And it ICEs when compile test case with it.
> > What I would suggest doing is to first start with running the testsuite
> > against the stage1 compiler before/after Peter's changes.  Sometimes
> > that'll turn up something useful and you can avoid debuging things
> > through stage2/stage3.
>
> Hi Jeff,
> Thanks for the suggestion! I could reproduce it with stage1 compiler.
>
>
>
> Hi Peter,
>
> I think I found the problem!
>
> As described in the PR, a hard register is used in
> an pre/post modify expression. The hard register is live, but updated.
> In this case, we should make it conflicting with all pseudos live at
> that point.  Does it make sense?

Err, that looks very much like a hack that manages to hide the issue.

Esp. adding conflicts in a loop that says "See which defined values die here."
is quite fishy.

Richard.

>
>
>
>
> It fixes the ICE of mis-compiled arm-linux-gnueabihf toolchain described in the
> PR.
>
> I attached the patch for discussion.  I haven't give a complete test on arm or
> any other targets, yet. (Probably need more adjusting)
>
> I will run arm and aarch64 regression test, cross and native.
>
> Regards,
> Renlin
>
> BTW, The pre/post modify expression is generated by auto_inc/auto_dec pass.
> somehow, it merges with hard register,  for example function argument registers.
> This optimization make the life for RA harder. Probably we don't want that pass
> too aggressive. @Wilco.
> (This IRA/LRA and the combiner change reveals a lot of issues,
> force us to work on it and improve the compiler :) .)
>
> gcc/ChangeLog:
>
> 2018-11-08  Renlin Li  <renlin.li@arm.com>
>          PR middle-end/87899
>         * lra-lives.c (process_bb_lives): Make hard register of INOUT
>         type conflict with all live pseudo.
>
>
>
>
>
>
>
> >
> >
> > jeff
> >