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

Vladimir Kozlov vladimir.kozlov at oracle.com
Wed Aug 6 16:44:40 UTC 2014


Good.

Thanks,
Vladimir

On 8/6/14 12:38 AM, Tobias Hartmann wrote:
> Hi Vladimir,
>
> thanks for the review.
>
> On 05.08.2014 18:47, Vladimir Kozlov wrote:
>> On 8/5/14 12:55 AM, Tobias Hartmann wrote:
>>> 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.
>>
>> I can't find Rickard's review. I am fine with this since BoolNode::Ideal() become too large.
>> cmp2 is not used, don't need to pass it. Declare cmp argument as SubNode* and pass cmp->as_Sub() so you don't need to
>> do it inside.
>
> Done. New webrev:
>
> http://cr.openjdk.java.net/~thartmann/8043284/webrev.03/
>
> Thanks,
> Tobias
>
>>
>> Thanks,
>> Vladimir
>>
>>>
>>> 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
>>>>>
>>>
>


More information about the hotspot-compiler-dev mailing list