RFR (M) 8042786: Proper fix for 8032566
Vladimir Kozlov
vladimir.kozlov at oracle.com
Thu May 15 20:57:41 UTC 2014
http://cr.openjdk.java.net/~kvn/8042786/webrev.01/
Only subnode.cpp was changed. Renamed variables and added missed overlap
check (hi_tr1 < lo_tr2).
I thought more about subtraction of minint. I think the long arithmetic
provides correct overflow answer for question about 2 type ranges. x
-(-maxint) == x + maxint will overflow for x > 0. We should get the same
overflow answer for x - minint because the subtraction result should be
at the same range as x - (-maxint). And for negative x the answer should
be 'no overflow'. Note, the numeric 32-bit result of x - minint is the
same when executing int arithmetic or after casting long result to int.
The difference only in overflow because in 32-bit -minint == minint.
Thanks,
Vladimir
On 5/14/14 10:58 PM, Vladimir Kozlov wrote:
> Thank you, John
>
> On 5/14/14 7:14 PM, John Rose wrote:
>> On May 14, 2014, at 5:26 PM, Vladimir Kozlov
>> <vladimir.kozlov at oracle.com> wrote:
>>
>>> http://cr.openjdk.java.net/~kvn/8042786/webrev/
>>> https://bugs.openjdk.java.net/browse/JDK-8042786
>>>
>>> 8032566 was fixed in jdk8 by disabling autobox elimination:
>>> -XX:-EliminateAutoBox.
>>>
>>> This fix will undo that change and fix the problem. In jdk9 8032566
>>> change was not applied before, I will push it first so that I could
>>> use the same 8042786 changeset for 8u backport.
>>
>> Nice change; reviewed.
>>
>> One comment: Surely there are better ways to differentiate the
>> lower-case versions of the identifiers "HI1" and "HIL" by relying on
>> "1" vs "l".
>> I suggest linking to prior "t1" as "t1_hi", "t1_lo".
>
> Okay. It is t11_lo and t12_lo.
>
>>
>> This optimization might also be beneficial on some signed comparisons,
>> notably (r1+r2)==0 where r1 and r2 are both [1..maxint].
>> So I suggest doing a follow-up application to CmpI.
>
> Agree, I will file RFE.
>
> About subtraction of [minint, x] which is converted into addition of
> [-x, -minint]. The concern was because -minint == minint in 32-bit int.
> I think it is fine in 64-bit long arithmetic which I used to detect
> overflow.
>
> What I missed is the overlap check (hi_r1 < lo_r2) for new type ranges.
> If they overlap we back to [minint, maxint] bottom type and can't do
> optimization.
>
> I will fix it and repeat testing tomorrow and will send new webrev.
>
>>
>> — John
>>
>
> Thanks,
> Vladimir
More information about the hotspot-compiler-dev
mailing list