RFR(XS): 8143307: Crash in C2 local code motion

[Lindenmaier, Goetz]

That's good, thanks a lot!
The recent change is running through our automatic testing tonight.
So far I hand-tested it with the bug-replay setup and some smaller
tests, there it works.

I also had added guarantee(e->in(1)->ideal_reg() == Op_RegFlags, ""), 
and that kicks in quite often.

Best regards,
  Goetz


> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Montag, 23. November 2015 17:08
> To: Lindenmaier, Goetz; hotspot compiler
> Subject: Re: RFR(XS): 8143307: Crash in C2 local code motion
> 
> Yes, this fix looks good. I will sponsor it.
> 
> Thanks,
> Vladimir
> 
> On 11/23/15 5:28 AM, Lindenmaier, Goetz wrote:
> > Hi Vladimir,
> >
> > thanks for looking at this!
> >
> >> Is this Itanium specific problem? Who produces flags for branch?
> >> I don't see Branch_loop node in .ad files.
> > We saw it in our zArch node.  "Branch_loop" is a pseudo-name for the
> > mach node I choose because the Z-name is too cryptic.
> > That node is similar to sparc:
> > instruct cmpI_reg_branchLoopEnd(cmpOp cmp, iRegI op1, iRegI op2, label
> labl, flagsReg icc) %{
> >    match(CountedLoopEnd cmp (CmpI op1 op2));
> > so the issue should also apply to openJDK.
> >
> >> Changes are fine but I am concern that could be other places where we
> >> assume that there is an input node which produces flags.
> > Oh, yes, for other nodes it's the compare that's checked.
> > I didn't grok that! Then, the cause of the error is a different one,
> > and the fix should check for RegFlags before dereferencing once
> > more.  See my adapted changeset:
> > http://cr.openjdk.java.net/~goetz/webrevs/8143307-
> lcmCrash/webrev.01/
> >
> > Best regards,
> >    Goetz.
> >
> >
> >
> >
> >>
> >> Thanks,
> >> Vladimir
> >>
> >> On 11/19/15 5:21 AM, Lindenmaier, Goetz wrote:
> >>> Hi,
> >>>
> >>> We see assert(i < _max) failed: oob: i=1, _max=1  in C2s local code
> motion.
> >>>
> >>> lcm.cpp:502 accesses e->in(1)->in(1) where e is an
> Op_CountedLoopEnd.
> >>>
> >>> e->in(1) should return a loop iteration node. In the failing case the
> >>> loop iteration node has been optimized out and replaced by a Proj,
> which
> >>> has no in(1).
> >>>
> >>> In detail, in the ideal world, I could observe the following
> >>> optimization steps:
> >>>
> >>> 1.) If( Proj_lt (Cmp(AddI(Proj_res(Call(...)), -1) LoadI(...) ) )
> >>>
> >>> 2.) CountedLoopEnd( Proj_lt (Cmp(AddI(Proj_res(Call(...)), -1)
> >>> LoadI(...) ) )
> >>>
> >>> 3.) CountedLoopEnd( Proj_le (Cmp(Proj_res(Call(...)), LoadI(...) ) )
> >>>
> >>> 4.) CountedLoopEnd( Proj_le(Cmp(x,y))) was matched into one mach
> >> node:
> >>>
> >>>        Branch_loopEnd(Proj_res(Call(...)), LoadI(...) )
> >>>
> >>> On pattern 4.) e->in(1)->in(1) fails.
> >>>
> >>> Fix: check whether e->in(1)->req() > 1.
> >>>
> >>> Please review this change. I please need a sponsor.
> >>>
> >>> http://cr.openjdk.java.net/~goetz/webrevs/8143307-
> >> lcmCrash/webrev.00/
> >>>
> >>> Best regards,
> >>>
> >>>     Goetz.
> >>>