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

Fri Nov 15 10:16:50 UTC 2019

Thanks for the reviews. I created a new webrev with the Reviewed-by line 
added to the changeset:

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

Could you sponsor the change for me and commit it into the repository?

Thanks,
Christoph

Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote on 2019-11-14 18:53:20:

> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
> To: christoph.goettschkes at microdoc.com
> Cc: hotspot-compiler-dev at openjdk.java.net
> Date: 2019-11-14 18:53
> Subject: Re: RFR: 8231954: [TESTBUG] Test compiler/codegen/
> TestCharVect2.java only works with server VMs.
> 
> +1
> 
> Vladimir
> 
> On 11/14/19 8:44 AM, Igor Ignatev wrote:
> > LGTM
> > 
> > — Igor
> > 
> >> On Nov 14, 2019, at 3:22 AM, christoph.goettschkes at microdoc.com 
wrote:
> >>
> >> 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
> >>>>>
> >>>>
> >>>
> >>
> > 
> 