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

[Vladimir Ivanov]

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/