[9] RFR(S): 8043284: Optimize signed integer comparison

[Vladimir Kozlov]

You are right about ConINode::make(), I forgot that it is Ideal() method.

The second condition is still in webrev:

 >> sub_tr1 == TypeInt::CC_GT && sub_tr2 == TypeInt::CC_LT

Thanks,
Vladimir

On 8/4/14 7:41 AM, Tobias Hartmann wrote:
> Hi Vladimir,
>
> thanks for the review. Please see comments inline.
>
> On 31.07.2014 18:43, Vladimir Kozlov wrote:
>> You missed cop == Op_CmpI check.
>
> Thanks, I added the check.
>
>> And add checks for r0 and r1 != TypeInt::INT (no bottom type) in addition to != NULL.
>
> Done.
>
>> You don't need t2 because there is already cmp2_type. Check cmp2_type for != TypeInt::INT too.
>
> Done.
>
>> Use as_Sub() (which is cast) instead of isa_Sub() because there is is_Sub() check at the beginning already.
>
> Done.
>
>> I don't see how second condition could happen because tr1 < tr2:
>>
>> sub_tr1 == TypeInt::CC_GT && sub_tr2 == TypeInt::CC_LT
>
> Of course, you are right. I removed the second condition.
>
>> Instead of ConINode::make() you can use phase->intcon().
>
> I first used phase->intcon() but this method may return a cached ConINode and hit the assert "Idealize should return new
> nodes, use Identity to return old nodes" in 'PhaseIterGVN::transform_old()'. That's why I directly create the ConINode.
>
> New webrev: http://cr.openjdk.java.net/~thartmann/8043284/webrev.01/
>
> Thanks,
> Tobias
>
>>
>> Thanks,
>> Vladimir
>>
>> On 7/31/14 5:35 AM, Tobias Hartmann wrote:
>>> Hi,
>>>
>>> please review the following patch that fixes JDK-8043284.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8043284
>>> Webrev: http://cr.openjdk.java.net/~thartmann/8043284/webrev.00/
>>>
>>> == Problem ==
>>> Similar to the previous implementation of unsigned integer comparison (see JDK- 8042786), signed integer comparison does
>>> only consider the general integer range [MinInt, MaxInt] instead of two ranges when comparing the result of an
>>> overflowing AddI/SubI. However, there are situations where one can prove that both resulting ranges are unequal to the
>>> tested value (see 'TestIntegerComparison::testSigned' in the webrev). In this case the comparison can be optimized.
>>>
>>> == Solution ==
>>> Similar to the fix for JDK-8042786 we compute both type ranges if the add/sub overflows and compare them to the tested
>>> value. If we can prove that the value is always unequal, we replace the BoolNode by true or false (depending on the
>>> test).
>>> A jtreg test is added that triggers both the unsigned as well as the signed optimization.
>>>
>>> == Testing ==
>>> - Jtreg test (TestIntegerComparison)
>>> - JPRT
>>>
>>> Thanks,
>>> Tobias
>