RFR: 8060036: C2: CmpU nodes can end up with wrong type information

[Tobias Hartmann]

On 21.05.2015 19:51, Vladimir Kozlov wrote:
> Since this fix 8060036 is already in JPRT queue lets use 8080156 to add the regression test.

Okay, I sent a new RFR with the regression test.

Thanks,
Tobias

> 
> Thanks,
> Vladimir
> 
> On 5/21/15 10:28 AM, Tobias Hartmann wrote:
>> Hi Andreas,
>>
>> I was working on JDK-8080156 [1] and just noticed that this is the same issue as JDK-8060036. The bug contains a small reproducer (see Ivan Gerasimov's comment), in case you want to add this as a regression test to your fix.
>>
>> Best,
>> Tobias
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8080156
>>
>> On 19.05.2015 12:56, Andreas Eriksson wrote:
>>> Hi,
>>>
>>> Could I please get two reviews for the following bug fix.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8060036
>>> http://cr.openjdk.java.net/~aeriksso/8060036/webrev.00/
>>>
>>> The problem is with type propagation in C2.
>>>
>>> A CmpU can use type information from nodes two steps up in the graph, when input 1 is an AddI or SubI node.
>>> If the AddI/SubI overflows the CmpU node type computation will set up two type ranges based on the inputs to the AddI/SubI instead.
>>> These type ranges will then be compared to input 2 of the CmpU node to see if we can make any meaningful observations on the real type of the CmpU.
>>> This means that the type of the AddI/SubI can be bottom, but the CmpU can still use the type of the AddI/SubI inputs to get a more specific type itself.
>>>
>>> The problem with this is that the types of the AddI/SubI inputs can be updated at a later pass, but since the AddI/SubI is of type bottom the propagation pass will stop there.
>>> At that point the CmpU node is never made aware of the new types of the AddI/SubI inputs, and it is left with a type that is based on old type information.
>>>
>>> When this happens other optimizations using the type information can go very wrong; in this particular case a branch to a range_check trap that normally never happened was optimized to always be taken.
>>>
>>> This fix adds a check in the type propagation logic for CmpU nodes, which makes sure that the CmpU node will be added to the worklist so the type can be updated.
>>> I've not been able to create a regression test that is reliable enough to check in.
>>>
>>> Thanks,
>>> Andreas