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

[Tobias Hartmann]

On 27.03.2017 17:43, Rob McKenna wrote:
> Approved pending successful nightly results.
> 
> Please follow the approval request template for future requests:
> 
> http://openjdk.java.net/projects/jdk8u/approval-template.html

Thanks, Rob. I'll do so!

Best regards,
Tobias

> On 27/03/17 11:17, Tobias Hartmann wrote:
>> Hi,
>>
>> could I please get approval to backport this fix to JDK 8u?
>>
>> The fix has been reviewed and was pushed to JDK 9:
>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-March/025891.html
>>
>> I'll wait for some nightly runs to complete before pushing this to JDK 8u.
>>
>> Thanks,
>> Tobias
>>
>> On 22.03.2017 13:31, 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
>>>