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

[Andreas Eriksson]

Thanks Vladimir!

On 2015-05-20 18:17, Vladimir Kozlov wrote:
> Looks good. I agree with explanation and fix.
>
> Thanks,
> Vladimir
>
> On 5/20/15 4:04 AM, Andreas Eriksson wrote:
>> Comments added, thanks.
>>
>> Can I have a Reviewer look at this as well?
>>
>> New webrev:
>> http://cr.openjdk.java.net/~aeriksso/8060036/webrev.01/
>>
>> - Andreas
>>
>> On 2015-05-19 20:24, Berg, Michael C wrote:
>>> Hi Andreas, could you add a comment to the push statement:
>>>
>>> // Propagate change to user
>>>
>>> And to Node* p declaration and init:
>>>
>>> // Propagate changes to uses
>>>
>>> So that the new code carries all similar information.
>>> Else the changes seem ok.
>>>
>>> -Michael
>>>
>>> -----Original Message-----
>>> From: hotspot-compiler-dev
>>> [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of
>>> Andreas Eriksson
>>> Sent: Tuesday, May 19, 2015 3:56 AM
>>> To: hotspot-compiler-dev at openjdk.java.net
>>> Subject: RFR: 8060036: C2: CmpU nodes can end up with wrong type
>>> information
>>>
>>> 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
>>