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

Dmitrij Pochepko dmitrij.pochepko at oracle.com
Thu Sep 1 14:28:57 UTC 2016


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

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



More information about the hotspot-compiler-dev mailing list