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

[Vladimir Ivanov]

Evgeniya,

Looks good!

BTW, why do you remove -XX:+IgnoreUnrecognizedVMOptions?

Best regards,
Vladimir Ivanov

On 9/18/14, 12:52 PM, Evgeniya Stepanova wrote:
> 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/