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

Tue Nov 19 10:38:42 UTC 2019

Hi Igor,

could you please sponsor this changeset and commit it for me into the 
repository?

https://cr.openjdk.java.net/~cgo/8231954/webrev.03/jdk-jdk.changeset

Thanks,
Christoph

Igor Ignatyev <igor.ignatyev at oracle.com> wrote on 2019-11-15 20:21:51:

> 
> 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
> >>>>> 
> >>>> 
> >>> 
> 