[8u] Request for approval: Backport of 8147548: need comprehensive fix for unconstrained ConvI2L with narrowed type

[Tobias Hartmann]

Hi Vladimir,

thanks for the review!

On 25.01.2016 18:49, Vladimir Kozlov wrote:
> RFR for 8u should use original 6675699 bug id and also use it in 8u the changeset comment:
> 
> https://bugs.openjdk.java.net/browse/JDK-6675699

Right, somehow I got confused because the backport issue was created explicitly. Sorry for that.

Here is the correct information (I leave the email subject for history):

6675699: need comprehensive fix for unconstrained ConvI2L with narrowed type
https://bugs.openjdk.java.net/browse/JDK-6675699
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-January/020810.html
http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/bfb7a8a004de

> parse2.cpp  I noticed that JDK9 change remove -1 from "num_cases-1". Was it intentional?

Yes, this is intended because the JDK 9 change uses Compile::conv_I2X_index() which already subtracts 1 from ikeytype->_hi:

4019   if (sizetype != NULL) index_max = sizetype->_hi - 1;

JDK 8u change uses C->constrained_convI2L() which does not change the ikeytype->_hi.

> graphKit.cpp fast_size_limit is used in jdk9 but not in 8u. Why?

Thanks for pointing that out. The JDK 8u fix should use 'fast_size_limit' as upper bound as well because it's more accurate than 'max_array_length'.

New webrev:
http://cr.openjdk.java.net/~thartmann/6675699_8u/webrev.01/

Best,
Tobias

> On 1/25/16 1:59 AM, Tobias Hartmann wrote:
>> Hi,
>>
>> please approve and review the following backport to 8u.
>>
>> 8147548: need comprehensive fix for unconstrained ConvI2L with narrowed type
>> https://bugs.openjdk.java.net/browse/JDK-8147548
>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-January/020810.html
>> http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/bfb7a8a004de
>>
>> The fix was pushed to hs-comp on January, 18 and nightly testing showed no problems. Unfortunately, the changes do not apply cleanly to 8u-dev because several enhancements that affect related code were not backported to 8. Here is a new webrev:
>> http://cr.openjdk.java.net/~thartmann/6675699_8u/webrev.00/
>>
>> I did additional performance and correctness testing to verify that the backport works fine and does not introduce a regression.
>>
>> Thanks,
>> Tobias
>>