RFR(S): 8146096 - [TEST BUG] compiler/loopopts/UseCountedLoopSafepoints.java Timeouts

[Vladimir Kozlov]

I forgot an other useful C2 flag -XX:LoopUnrollLimit=0  to avoid loop 
iteration splitting. Otherwise you will get several loops (pre-, main- 
post-).

Thanks,
Vladimir

On 9/1/16 10:40 AM, Vladimir Kozlov wrote:
> Yes, this looks good.
>
> On 9/1/16 7:28 AM, Dmitrij Pochepko wrote:
>> Thank you for attentive review.
>>
>>
>>> Wow! You wrote "parser" to C2 Ideal graph.
>>>
>>> // now, find SafePoint->CountedLoopEnd edge
>>>
>>> Actually CountedLoopEnd  input edge should point to SafePoint. Not
>>> reverse. You should search from LoopEnd up.
>> fixed
>>>
>>>  115    SafePoint    ===  112  1  107  1  1  10  110  [[ 116 ]]
>>> SafePoint !orig=76 !jvms: SimpleTest::testMethod @ bci:21
>>>  116    CountedLoopEnd    ===  115  105  [[ 117  118 ]] [lt]
>>> P=0.500000, C=6633.000000 !orig=97,[80] !jvms: SimpleTest::testMethod
>>> @ bci:5
>>>
>>> The test is too simple. Without UseCountedLoopSafepoints the loop is
>>> folded to accum += 100. You need a little more complex.
>> I've changed increment to combination of different shifts
>>>
>>> Also test should be run only with C2 (Tiered should be off). I don't
>>> think require vm.opt.TieredStopAtLevel is the same as
>>> -XX:-TieredCompilation.
>> Requires expression just help to ensure level 4 is available, because
>> testMethod specifically compiled on level=4 using WhiteBox
>> (CompilerWhiteBoxTest.COMP_LEVEL_FULL_OPTIMIZATION).
>> This differs from original test idea to disable tiered compilation and
>> trigger compilation by running cycle 2 billion times, which made it a
>> long "stress" test. Now that method compiled via WhiteBox, this test
>> became much more faster
>
> Got it.
>
> Thanks,
> Vladimir
>
>>
>> Please take a look at v03:
>> http://cr.openjdk.java.net/~dpochepk/8146096/webrev.03/
>>
>> Thanks,
>> Dmitrij
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 8/31/16 12:03 PM, Dmitrij Pochepko wrote:
>>>> Hi,
>>>>
>>>> Please take a look at v02.
>>>>
>>>> I've rewritten this test. Now it launch vm with
>>>> -XX:+UseCountedLoopSafepoints (restricting compilation to tested method
>>>> with single simple counted loop) and checks that output of
>>>> -XX:+PrintIdeal have edge SafePoint -> CountedLoopEnd
>>>>
>>>> Then, launch the same with -XX:-UseCountedLoopSafepoints and checks
>>>> that
>>>> there is no such edge.
>>>>
>>>> I've tested fix via rbt on all platforms.
>>>>
>>>> webrev: http://cr.openjdk.java.net/~dpochepk/8146096/webrev.02/
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Dmitrij
>>>>
>>>> On 29.08.2016 19:28, Vladimir Kozlov wrote:
>>>>> I am not against marking test as stress but I think the test itself is
>>>>> not good. It should be rewrote. I added comment to the JBS with
>>>>> discussion during original 8146096 RFR.
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 8/29/16 7:08 AM, Dmitrij Pochepko wrote:
>>>>>> Hi,
>>>>>>
>>>>>> please review small fix for 8146096 - [TEST BUG]
>>>>>> compiler/loopopts/UseCountedLoopSafepoints.java Timeouts
>>>>>>
>>>>>>
>>>>>> Test timeouts on slow platforms, so, this fix adds execution control
>>>>>> with respect to elapsed time. Also, test marked as stress.
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~dpochepk/8146096/webrev.01/
>>>>>>
>>>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8146096
>>>>>>
>>>>>>
>>>>>> I've tested this fix on linux-amd64.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Dmitrij
>>>>>>
>>>>
>>