[11] RFR(S): 8202949: C2: assert(false) failed: Bad graph detected in build_loop_late

[Tobias Hartmann]

Hi Roland,

I've looked at this again in more detail and found the actual root cause to be overunrolling of the
inner for-loop. Removal of the CastII/ConvI2L is correct and webrev.00 only hides the problem
because of a slightly different graph shape. Sorry for the confusion.

It's basically the same problem I've already fixed with JDK-8159016 [1] some time ago but my fix is
incomplete because it does not work for loops with negative stride.

What happens is that the inner for-loop is unrolled 4 times because we fail to determine the
constant lower and upper bound (6,8] when computing the trip count. After unrolling, the range check
dependent CastII/ConvI2L emitted for the iArr access become TOP because index 'j' is out of these
bounds. As a result, the memory graph is corrupted with nodes consuming memory from a non-dominating
(dead) region.

The fix is to handle a negative stride:
http://cr.openjdk.java.net/~thartmann/8202949/webrev.01/

I've also merged the test with the existing over-unrolling test.

Thanks,
Tobias

[1]
https://bugs.openjdk.java.net/browse/JDK-8159016
http://cr.openjdk.java.net/~thartmann/8159016/webrev.00/

On 23.05.2018 16:06, Roland Westrelin wrote:
> 
>> Right, I think the point is more that the type of the ConvI2L is narrowed which is usually prevented
>> by the CastII where we do not allow the type to be narrowed in case it is range check dependent.
> 
> Can you point me to the code you're referring to (code that narrows
> ConvI2L but not CastII)?
> 
> AFAICT, ConvI2LNode::Value() and ConstraintCastNode::Value() are mostly
> identical.
> 
> I thought the benefit of the CastII is that it carries a control
> dependence on the range check.
> 
> Roland.
>