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

[Vladimir Kozlov]

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.
>>>