RFR: 8231954: [TESTBUG] Test compiler/codegen/TestCharVect2.java only works with server VMs.

Fri Nov 15 19:21:51 UTC 2019

I'd also add that unfortunately it's not always right to add '@requirers vm.compiler2.enabled' to a test just b/c it uses some c2-only flags. there are cases when such tests aren't just still valid w/o these flags, but are capable to spot bugs, on the other hand, tests which have multiple runs w/ different values of c2-only (or c1-only) flags can be split to reduce wasted time. that's to say decision on whenever @requires is a right thing should be done on test-per-test basis.

as 8231954 was only about TestCharVect2 test, I suggest we push Christoph's webrev.03 and file an RFE to deal w/ other tests, or retrofit 8228493 to talk not only about non-product flags but also about c2/c1-only flags and use it as an umbrella for discussion/work-tracking.

-- Igor

> On Nov 15, 2019, at 11:11 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
> Note, compiler/c2 and compiler/c1 was misleading naming for tests directories which is nothing to do with C1 and C2 JIT compilers. They are simply 2 groups of tests we split so they can be executed in parallel in reasonable time.
> 
> Vladimir
> 
> On 11/14/19 7:35 PM, Yang Zhang (Arm Technology China) wrote:
>> Hi Christoph, Igor, Vladimir,
>> Thanks very much for your fix. After discussion, we have got a better solution for this issue. Do we need to change the following files in which MaxVectorSize option is used?
>> [1] https://hg.openjdk.java.net/jdk/jdk/file/4dbdb7a8fa75/test/hotspot/jtreg/compiler/vectorization/TestNaNVector.java
>> [2] https://hg.openjdk.java.net/jdk/jdk/file/4dbdb7a8fa75/test/hotspot/jtreg/compiler/vectorization/TestPopCountVector.java
>> [3] https://hg.openjdk.java.net/jdk/jdk/file/4dbdb7a8fa75/test/hotspot/jtreg/compiler/c2/cr6340864
>> Ps. For [3],  it locates in c2 directory. So I'm not sure whether they will be excluded in jtreg test with client mode.
>> Regards
>> Yang
>> -----Original Message-----
>> From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> On Behalf Of christoph.goettschkes at microdoc.com
>> Sent: Thursday, November 14, 2019 7:21 PM
>> To: vladimir.kozlov at oracle.com; igor.ignatyev at oracle.com
>> Cc: hotspot-compiler-dev at openjdk.java.net
>> Subject: Re: RFR: 8231954: [TESTBUG] Test compiler/codegen/TestCharVect2.java only works with server VMs.
>> Thanks for your feedback, this resolves my concerns and I am happy with the solution. I integrated the suggestions from Vladimir, here is the latest webrev:
>> https://cr.openjdk.java.net/~cgo/8231954/webrev.02/
>> I re-tested and it works as expected.
>> Please give your consent if this is fine for you as well.
>> -- Christoph
>> Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote on 2019-11-13 20:32:18:
>>> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
>>> To: Igor Ignatyev <igor.ignatyev at oracle.com>,
>> christoph.goettschkes at microdoc.com
>>> Cc: hotspot compiler <hotspot-compiler-dev at openjdk.java.net>
>>> Date: 2019-11-13 20:32
>>> Subject: Re: RFR: 8231954: [TESTBUG] Test compiler/codegen/
>>> TestCharVect2.java only works with server VMs.
>>> 
>>> On 11/13/19 11:11 AM, Igor Ignatyev wrote:
>>>> @Christoph,
>>>> 
>>>> webrev.01 looks good to me.
>>>> I always thought that jvmci feature can be built only when compiler2
>>> feature is enabled, however src/hotspot/share/jvmci/jvmci_globals.hpp
>>> suggests that jvmci can be used w/o compiler2; I don't think we have
>>> ever build/test, let alone support, this configuration.
>>>> 
>>>> @Vladimir,
>>>> did/do we plan to support compiler1 + jvmci w/o compiler2
>> configuration?
>>> 
>>> Yes. It could be configuration when we start looking on replacing C1
>>> with Graal. I think several people were interested in "Client VM" like
>>> configuration.
>>> Also Server configuration without C2 (with Graal or other jvmci
>>> compiler) which would be out configuration in a future.
>>> 
>>> But I would prefer to be more explicit in these changes:
>>> 
>>> @requires vm.compiler2.enabled | vm.graal.enabled
>>> 
>>> Thanks,
>>> Vladimir
>>> 
>>>> 
>>>> Thanks,
>>>> -- Igor
>>>> 
>>>>> On Nov 13, 2019, at 4:42 AM, christoph.goettschkes at microdoc.com
>> wrote:
>>>>> 
>>>>> Hi Igor,
>>>>> 
>>>>> thanks for your explanation.
>>>>> 
>>>>> Igor Ignatyev <igor.ignatyev at oracle.com> wrote on 2019-11-12
>> 20:40:46:
>>>>> 
>>>>>> we are trying to get rid of IgnoreUnrecognizedVMOptions in our
>> tests, as
>>>>>> in most cases, it causes wasted compute time (as in this test) and
>> can
>>>>>> also lead to wrong/deprecated/deleted flags sneaking into the
>> testbase
>>>>> 
>>>>> Agreed. I also wanted to discuss this, since I think that your
>> solution
>>>>> is better than mine, but at the same time, I saw possible problems
>> with
>>>>> it, see below.
>>>>> 
>>>>>> as '@requires vm.flavor == "server"' filters configurations based
>>>>>> vm build type, it will still allow execution on JVM w/ JVMCI and
>>>>>> when
>> JVMCI
>>>>>> compiler is selected, as it will still be Server VM build. so, in
>>>>>> a sense, the test will be w/ JVMCI in the same way as w/ your
>> approach.
>>>>> 
>>>>> My concern is not about server VMs with JVMCI, but client VMs with
>> JVMCI
>>>>> enabled. Is this a valid configuration? The MaxVectorSize option is
>>>>> defined in [1] as well as in [2], so for me it looks like
>> MaxVectorSize
>>>>> can be used for any VM variant as long as JVMCI is enabled. The
>>>>> configure script also states that both compilers are possible (if
>>>>> you configure with --with-jvm-features='jvmci'):
>>>>> 
>>>>> configure: error: Specified JVM feature 'jvmci' requires feature
>>>>> 'compiler2' or 'compiler1'
>>>>> 
>>>>> Should maybe the requires tag "vm.jvmci" be used as well, like:
>>>>> 
>>>>>     @requires vm.flavor == "server" | vm.jvmci
>>>>> 
>>>>>> this is the known limitation of jtreg/@requires, and our current
>>>>>> way
>> to
>>>>>> workaround it is to split a test description based on @requires
>> values
>>>>> 
>>>>> Yes, if the @requires tag is used, splitting up the test looks like
>>>>> a
>> good
>>>>> idea. I didn't know that it is possible to have multiple test
>> descriptions
>>>>> in one test file.
>>>>> 
>>>>> I created a new webrev with the new ideas:
>>>>> 
>>>>> https://cr.openjdk.java.net/~cgo/8231954/webrev.01/
>>>>> 
>>>>> I tested with an amd64 client and server VM and it looks good. I am
>>>>> currently unable to build a client VM with JVMCI enabled, hence no
>> test
>>>>> for that yet. I get compile errors and as soon as I resolve those,
>>>>> runtime errors occur. Before I look into that, I would like to know
>> if
>>>>> client VMs with JVMCI enabled are supported or not.
>>>>> 
>>>>> Thanks,
>>>>> Christoph
>>>>> 
>>>>> [1]
>>>>> https://hg.openjdk.java.net/jdk/jdk/file/dc1899bb84c0/src/hotspot/
>>> share/opto/c2_globals.hpp
>>>>> 
>>>>> [2]
>>>>> https://hg.openjdk.java.net/jdk/jdk/file/dc1899bb84c0/src/hotspot/
>>> share/jvmci/jvmci_globals.hpp
>>>>> 
>>>> 
>>> 