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

[Tobias Hartmann]

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