RFR: 8038098: [TESTBUG] remove explicit set build flavor from hotspot/test/compiler/* tests

[Evgeniya Stepanova]

Hi Vladimir,

Webrev was changed according to your notices
Please see http://cr.openjdk.java.net/~iignatyev/eistepan/8038098/webrev.02/

Thanks,
Jane
On 17.09.2014 13:50, Vladimir Ivanov wrote:
> Evgeniya,
>
> Please, keep the following configurations in test/compiler/stable tests:
>   * -XX:-TieredCompilation
>   * -XX:+TieredCompilation -XX:TieredStopAtLevel=1
>   * -XX:+-FoldStableValues
>
> Since there's a difference in behavior between C1 & C2 w.r.t @Stable, 
> the tests assume there's only C1 or C2 working during test execution 
> (see StableConfiguration). Otherwise, they fail.
>
> Best regards,
> Vladimir Ivanov
>
> On 9/17/14, 12:20 PM, Vladimir Kozlov wrote:
>> Wow! You really did not hold back :)
>>
>> I think you should keep -XX:TieredStopAtLevel=1 runs in stable tests. On
>> some platforms (Solaris, MacOSX) we don't have Client VM and pure C1
>> compilation anymore. In tiered C1 does mostly tier 3 compilation. The
>> only way to test pure C1 on such platforms is with TieredStopAtLevel=1.
>> You don't need other options, it has effect only with Tiered 
>> Compilation:
>>
>>     *                   -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
>> -Xcomp
>>     *                   -XX:TieredStopAtLevel=1 -XX:+FoldStableValues
>>
>> Please, restore 2 @run commands with -XX:+FoldStableValues and
>> -XX:-FoldStableValues with TieredStopAtLevel=1.
>>
>> Yes, in non-tiered and Client VM runs we will waste these 2 runs but I
>> don't want to lost the coverage.
>>
>> Thanks,
>> Vladimir
>>
>>
>> On 9/17/14 12:16 AM, Evgeniya Stepanova wrote:
>>> Hi all!
>>>
>>> I've changed tests mentioned: VM flavor and TieredCompilation options
>>> removed since these are rotated values in the
>>> testcycle.
>>> Please see
>>> http://cr.openjdk.java.net/~iignatyev/eistepan/8038098/webrev.01/
>>>
>>> Thanks,
>>> Jane
>>>
>>> On 16.09.2014 19:57, Vladimir Kozlov wrote:
>>>> On 9/16/14 4:51 AM, Filipp Zhinkin wrote:
>>>>> Igor, Vladimir,
>>>>>
>>>>> RTM tests will be skipped if the class of JVM used for a test
>>>>> execution is not 'server'.
>>>>> Such check is done before the '-server' is passed in RTMTestBase, so
>>>>> '-server'
>>>>> removal won't increase coverage.
>>>>
>>>> Okay, it make sense to have it here then.
>>>> I agree that it does not make sense to run RTM tests  with client VM.
>>>>
>>>> Vladimir
>>>>
>>>>>
>>>>> If we'll remove it, then I don't think that it will affect test
>>>>> execution:
>>>>> if JVM class was not explicitly specified on the command line, then
>>>>> a java launcher will choose it depending on available physical memory
>>>>> and available processors count. So if JVM was 'server' when we're
>>>>> checking
>>>>> its class, then it's unlikely that JVM started in RTMTestBase will
>>>>> become 'client'.
>>>>>
>>>>> But if we'll leave '-server' then tests continue to work as expected
>>>>> even
>>>>> if a java launcher will start choosing JVM class using some other
>>>>> policy.
>>>>>
>>>>> Thanks,
>>>>> Filipp.
>>>>>
>>>>>
>>>>> On 09/16/2014 02:35 PM, Igor Ignatyev wrote:
>>>>>> Vladimir,
>>>>>> I doubt that we can remove -server from
>>>>>> compiler/testlibrary/rtm/RTMTestBase.java. AFAIR, it requires
>>>>>> -server, since
>>>>>> it uses server only flag 'UseRTMLocking'
>>>>>>
>>>>>> Filipp,
>>>>>> can you please verify it?
>>>>>>
>>>>>> Igor
>>>>>>
>>>>>> On 09/15/2014 09:42 PM, Vladimir Kozlov wrote:
>>>>>>> Hi Evgeniya
>>>>>>>
>>>>>>> What about LoadWithMask.java test?
>>>>>>>
>>>>>>> Please, simple remove '-server' option. We always do testing with
>>>>>>> server
>>>>>>> JVM so we can't miss the bug if it returns. We did go through such
>>>>>>> cleanup in tests before and did not have any problems.
>>>>>>>
>>>>>>> Flag -XX:-TieredCompilation may affect bug reproduction. That is
>>>>>>> why we
>>>>>>> specify it sometimes.
>>>>>>> In general we can remove it too since we run Nightly in both
>>>>>>> configurations with and without Tiered. But since it does not 
>>>>>>> affect
>>>>>>> what JVM is used we keep it.
>>>>>>>
>>>>>>> In compiler tests we should not specify neither -server or 
>>>>>>> -client to
>>>>>>> make sure the correct JVM is tested. This is one of our rules 
>>>>>>> when we
>>>>>>> add tests. Recently we added tests which have -server specified:
>>>>>>>
>>>>>>> compiler/IntegerArithmetic/TestIntegerComparison.java
>>>>>>> compiler/testlibrary/rtm/RTMTestBase.java
>>>>>>> compiler/uncommontrap/UncommonTrapStackBang.java
>>>>>>>
>>>>>>> compiler/stable/* test have  @run command with both -server and
>>>>>>> -client
>>>>>>> configurations.
>>>>>>>
>>>>>>> All these tests should be cleaned up.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Vladimir
>>>>>>>
>>>>>>> On 9/15/14 7:11 AM, Evgeniya Stepanova wrote:
>>>>>>>> Hi Filipp,
>>>>>>>>
>>>>>>>> I want to keep first string "as is" because it is a regression 
>>>>>>>> test
>>>>>>>> and if we just remove -server option, it is possible
>>>>>>>> that bug have returned, but we won't find it before release 
>>>>>>>> because
>>>>>>>> options will not compose in the right way.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> The test is very fast, so the second run will not take significant
>>>>>>>> time, but we will be sure that there is no regression.
>>>>>>>>
>>>>>>>> As for C2-specific options, I agree. I've removed -client and
>>>>>>>> TieredCompilation options to use jtreg submitted.
>>>>>>>>
>>>>>>>> Please see updated diff:
>>>>>>>> http://cr.openjdk.java.net/~iignatyev/eistepan/8038098/webrev.00/
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Jane
>>>>>>>> On 15.09.2014 16:01, Filipp Zhinkin wrote:
>>>>>>>>> Hi Jane,
>>>>>>>>>
>>>>>>>>> mail's summary does not match the bug's name.
>>>>>>>>>
>>>>>>>>> As about your fix: I thought that such configs as client/server
>>>>>>>>> and tired/non-tired should be rotated in nightly, aren't it?
>>>>>>>>>
>>>>>>>>> Maybe it does more sense to just remove '-server' option?
>>>>>>>>> With '-client' the test may fail during test runs that were 
>>>>>>>>> started
>>>>>>>>> with some C2-specific options, like UseRTMLocking.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Filipp.
>>>>>>>>>
>>>>>>>>> On 09/15/2014 03:42 PM, Evgeniya Stepanova wrote:
>>>>>>>>>> Hi all!
>>>>>>>>>>
>>>>>>>>>> Please review fix for 8038098
>>>>>>>>>>
>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8038098
>>>>>>>>>>
>>>>>>>>>> Problem: Since we have inlining in C1 now, we should check that
>>>>>>>>>> issue 8031743 will not appear in C1.
>>>>>>>>>> Solution: Added one more run in test for C1 inlining checking
>>>>>>>>>>
>>>>>>>>>> A am also need someone to push changes.
>>>>>>>>>>
>>>>>>>>>> Diff is in the attachments.
>>>>>>>>>> I've also included diff to this letter since it is very small.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Jane
>>>>>>>>>>
>>>>>>>>>> diff -r 087443edb74a test/compiler/codegen/LoadWithMask2.java
>>>>>>>>>> --- a/test/compiler/codegen/LoadWithMask2.java Mon Sep 15
>>>>>>>>>> 08:08:22 2014 +0200
>>>>>>>>>> +++ b/test/compiler/codegen/LoadWithMask2.java Mon Sep 15
>>>>>>>>>> 14:35:50 2014 +0400
>>>>>>>>>> @@ -26,6 +26,7 @@
>>>>>>>>>>   * @bug 8031743
>>>>>>>>>>   * @summary loadI2L_immI broken for negative memory values
>>>>>>>>>>   * @run main/othervm -server -Xbatch -XX:-TieredCompilation
>>>>>>>>>> -XX:CompileCommand=compileonly,*.foo* LoadWithMask2
>>>>>>>>>> + * @run main/othervm -client -Xbatch -XX:+TieredCompilation
>>>>>>>>>> -XX:CompileCommand=compileonly,*.foo* LoadWithMask2
>>>>>>>>>>   *
>>>>>>>>>>   */
>>>>>>>>>>  public class LoadWithMask2 {
>>>>>>>>>
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> /Evgeniya Stepanova/
>>>>>
>>>
>>> -- 
>>> /Evgeniya Stepanova/

-- 
/Evgeniya Stepanova/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140918/5ad22129/attachment-0001.html>