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

[Vladimir Kozlov]

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
>