[9] RFR(S): 8046268: compiler/whitebox/ tests fail : must be osr_compiled

[Tobias Hartmann]

Hi Igor,

thanks for the review.

On 08.10.2014 11:18, Igor Ignatyev wrote:
> Tobias,
>
> On 10/08/2014 12:29 PM, Tobias Hartmann wrote:
>> Hi Vladimir,
>>
>> thanks for the review.
>>
>> On 03.10.2014 19:20, Vladimir Kozlov wrote:
>>> Nightly runs done with -Xcomp. Make sure your changes still work.
>>
>> I did some more testing on JPRT with -Xcomp and also with
>> -XX:-TieredCompilation. No failures (the results are linked in the
>> comment section). Most tests explicitly specify '-Xmixed'.
>>
>>> 200 for tiered may be too high. C1 compilation triggered before that,
>>> please check.
>>
>> Yes, that's intended. After some more testing I even increased the value
>> to 2000 to make sure that the loop exit branch probability is high
>> enough and we never add an uncommon trap. If the method is compiled
>> during warmup the following deoptimization ensures that we throw away
>> the compiled version and an OSR compilation is triggered:
>>
>>   551             // Make sure method is not (yet) compiled
>>   552             deoptimize(m);
> no, it doesn't since you call 'WhiteBox.getWhiteBox().deoptimizeMethod(e,
> false);' where false means non-osr.

Yes, that's intended. We want to trigger an OSR compilation after the warmup. 
Therefore we remove all non-OSR versions by deoptimizing them. If the warmup 
triggers an OSR compilation (which is really unlikely because the loop is 
executed for only one iteration each) that's fine as well.

I added some comments to make this more explicit.

>>
>> I also added code to wait for a potential background compilation (see
>> 'Helper.deoptimize') because the method may still be in the compile queue.
> we already have 'CompilerWhiteBoxTest::waitBackgroundCompilation', I think it'd
> be better to add an argument to it and make it static, but leave instance method
> w/o args which calls it w/ method.

Done. New webrev:

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

Thanks,
Tobias

>>
>> The fixed tests pass on all platforms with and without -Xcomp.
>>
>> New webrev: http://cr.openjdk.java.net/~thartmann/8046268/webrev.01/
>>
>> Thanks,
>> Tobias
>>
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 10/3/14 2:22 AM, Tobias Hartmann wrote:
>>>> Hi,
>>>>
>>>> please review the following patch.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8046268
>>>> Webrev: http://cr.openjdk.java.net/~thartmann/8046268/webrev.00/
>>>>
>>>> == Problem ==
>>>> The whitebox tests assume that after the invocation of
>>>> 'CompilerWhiteBoxTest.compile()' the method is always compiled
>>>> and 'CompilerWhiteBoxTest.checkCompiled()' will return true. For OSR
>>>> compilations [1] this is does not hold. For
>>>> example, in case of the following events:
>>>>
>>>> 1) The test invokes 'CompilerWhiteBoxTest.compile()' to OSR compile
>>>> 'SimpleTestCase.Helper.osrMethod()'.
>>>> 2) OSR compilation is performed at level 4. An uncommon trap is added
>>>> after the loop because profiling never reached
>>>> that point. Execution continues with the OSR version.
>>>> 3) Deoptimization is triggered because the loop finishes (reason:
>>>> 'unstable if'). The OSR nmethod is made non-entrant
>>>> and removed from the OSR list (see 'nmethod::invalidate_osr_method').
>>>> 4) The test uses CompilerWhiteBoxTest.checkCompiled() to make sure the
>>>> method is OSR compiled. Because the OSR nmethod
>>>> was removed from the list, check compiled returns false (see
>>>> WB_IsMethodCompiled).
>>>>
>>>> This was caused by the fix for JDK-8030976 [2] because we now trust
>>>> the profile information more and add uncommon traps
>>>> to untaken paths. In this case we add an uncommon trap after the loop
>>>> exit:
>>>>
>>>> private int osrMethod() {
>>>>    int result = 0;
>>>>    for (long i = 0; i < CompilerWhiteBoxTest.BACKEDGE_THRESHOLD; ++i) {
>>>>      result += method();
>>>>    }
>>>>    return result;
>>>> }
>>>>
>>>> == Solution ==
>>>> I added code to 'warm up' the methods before triggering OSR
>>>> compilation by executing them a limited number of times.
>>>> Like this, the profile information marks the loop exit as taken and we
>>>> don't add an uncommon trap.
>>>>
>>>> == Testing ==
>>>> - Executed failing tests on JPRT
>>>>
>>>> Thanks,
>>>> Tobias
>>>>
>>>>
>>>> [1] OSR compiled methods:
>>>> compiler.whitebox.SimpleTestCase.Helper.OSR_CONSTRUCTOR
>>>> compiler.whitebox.SimpleTestCase.Helper.OSR_METHOD
>>>> compiler.whitebox.SimpleTestCase.Helper.OSR_STATIC
>>>>
>>>> [2] https://bugs.openjdk.java.net/browse/JDK-8030976