[8u, 9] RFR(S): 8177095: Range check dependent CastII/ConvI2L is prematurely eliminated

[Vladimir Ivanov]

Looks good.

Best regards,
Vladimir Ivanov

PS: after finishing JDK-8154831 I thought about (but never finished, 
unfortunately) implementing verification logic for range check dependent 
cast nodes to ensure they disappear in lockstep with the range check 
they depend on. It would make diagnosing such problems much easier.

On 3/22/17 3:31 PM, Tobias Hartmann wrote:
> Hi,
>
> please review the following patch:
> https://bugs.openjdk.java.net/browse/JDK-8177095
>
> JDK 9:
> http://cr.openjdk.java.net/~thartmann/8177095/webrev.00/
> JDK 8u:
> http://cr.openjdk.java.net/~thartmann/8177095_8u/webrev.00/
>
> A CastII (or a ConvI2L before the fix for JDK-6675699 [1]) with a narrow type that represents the index of an array access is eliminated because its type range does not intersect with the input range. This happens because the node is pushed upwards through an AddI. However, the control path to the array access is not eliminated leaving the graph in a corrupted state. We either crash during loop optimizations because we use a value from a non-dominating region or we crash in the register allocator because a Phi has a TOP input (and therefore an undefined live range).
>
> For more details see changes in TestLoopPeeling.java:
> Load1 and the corresponding range check are moved out of the loop and both are used after the old loop and the peeled iteration exit. For the peeled iteration, storeIndex is always Integer.MIN_VALUE and for the old loop it is 0. Hence, the merging phi has type int:<=0. Load1 reads the array at index ConvI2L(CastII(AddI(storeIndex, -1))) where the CastII is range check dependent and has type int:>=0. The CastII gets pushed through the AddI and its type is changed to int:>=1 which does not overlap with the input type of storeIndex (int:<=0). The CastII is replaced by TOP causing a cascade of other eliminations. Since the control path through the range check CmpU(AddI(storeIndex, -1)) is not eliminated, the graph is in a corrupted state. We fail once we merge with the result of Load2 because we get data from a non-dominating region.
>
> I attempted to fix this issue with JDK-6675699 but missed these cases. The problem is similar to JDK-8154831 [2] and affects the latest JDK 6, 7, 8 and 9.
>
> I disabled narrowing of range check dependent CastIIs (either through the CastII(AddI) optimization or through CastIINode::Ideal).
>
> Tested with customer provided reproducer, regression test and RBT (running).
>
> Thanks,
> Tobias
>
> [1] https://bugs.openjdk.java.net/browse/JDK-6675699
> [2] https://bugs.openjdk.java.net/browse/JDK-8154831
>