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

[Tobias Hartmann]

Hi Vladimir,

thanks for the review.

> The second condition is still in webrev:
>
> >> sub_tr1 == TypeInt::CC_GT && sub_tr2 == TypeInt::CC_LT

Sorry, I forgot to update the webrev after removing the condition.

As Rickard suggested, I moved the code into a separate method called 
'fold_cmpI'. If you think that the method has too many arguments, we 
could reduce them to '(PhaseGVN* phase, Node* cmp)' and re-compute the 
other values locally.

New webrev: http://cr.openjdk.java.net/~thartmann/8043284/webrev.02/

Thanks,
Tobias

>
> 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
>>