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

[Roland Westrelin]

> I did it that way because all the other
> 
> if (...)
>  worklist.push(p)
> 
> in that function omit the curly braces.
> Should I have them in this change anyway?

Some c2 code doesn’t (yet) follow the current style. The usual practice is to: 

1) use the current style for new code
2) fix the style around what you’ve modified

>> I can sponsor it if you need.
> 
> That would be very helpful, thanks.

Can you send me a changeset (hg export) with the properly formatted mercurial comment?
http://openjdk.java.net/guide/producingChangeset.html
section "Formatting a Changeset Comment”
Can you also make sure you’ve run jcheck.

Thanks,
Roland.

> 
> - Andreas
> 
>> 
>> Roland.
>> 
>>> - 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
>