[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    RE: RFR: 8245717: VM option "-XX:EnableJVMCIProduct" could not be repetitively enabled
From:       Xiaohong Gong <Xiaohong.Gong () arm ! com>
Date:       2020-05-29 2:43:21
Message-ID: VI1PR08MB5328062D7EE3380982385007F58F0 () VI1PR08MB5328 ! eurprd08 ! prod ! outlook ! com
[Download RAW message or body]

Hi Vladimir,

> Looks good to me.

Thanks so much for your reviewing! 

Best Regards,
Xiaohong Gong

-----Original Message-----
From: Vladimir Kozlov <vladimir.kozlov@oracle.com> 
Sent: Friday, May 29, 2020 12:45 AM
To: Xiaohong Gong <Xiaohong.Gong@arm.com>; David Holmes <david.holmes@oracle.com>; \
                hotspot-compiler-dev@openjdk.java.net; \
                hotspot-runtime-dev@openjdk.java.net
Cc: nd <nd@arm.com>
Subject: Re: RFR: 8245717: VM option "-XX:EnableJVMCIProduct" could not be \
repetitively enabled

Looks good to me.

Thanks,
Vladimir K

On 5/27/20 1:02 AM, Xiaohong Gong wrote:
> Hi David,
> 
> > On 26/05/2020 4:57 pm, Xiaohong Gong wrote:
> > > Hi,
> > > 
> > > Could you please help to review this simple patch? It fixes the
> > issue
> > > that JVM crashes in debug mode when the vm option
> > "-XX:EnableJVMCIProduct" is enabled repetitively.
> > > 
> > > JBS: https://bugs.openjdk.java.net/browse/JDK-8245717
> > > Webrev: http://cr.openjdk.java.net/~xgong/rfr/8245717/webrev.00/
> > > 
> > > Repetitively enabling the vm option "-XX:+EnableJVMCIProduct" in
> > the
> > > command line makes the assertion fail in debug mode:
> > "assert(is_experimental(), sanity)".
> > > 
> > > It happens when the VM iteratively parses the options from
> > command
> > > line. When the matched option is "-XX:+EnableJVMCIProduct", the
> > > original experimental JVMCI flags will be converted to product
> > mode,
> > > with the above assertion before it. So if all the JVMCI flags
> > have
> > > been converted to the product mode at the first time parsing
> > "-XX:+EnableJVMCIProduct", the assertion will fail at the second
> > time it is parsed.
> > > 
> > > A simple fix is to just ignoring the conversion if this option
> > has been parsed.
> > 
> > Seems a reasonable approach given the already complex handling of
> > this flag.
> > 
> > > Testing:
> > > Tested jtreg
> > > hotspot::hotspot_all_no_apps,jdk::jdk_core,langtools::tier1
> > > and jcstress:tests-custom, and all tests pass without new
> > failure.
> > 
> > I think adding a regression test in
> > ./compiler/jvmci/TestEnableJVMCIProduct.java would be appropriate.
> 
> Thanks for your review and it's a good idea to add the regression test.
> I have added the test in the new patch:
> http://cr.openjdk.java.net/~xgong/rfr/8245717/webrev.01/ .
> 
> Could you please take a look at it? Thank you!
> 
> Thanks,
> Xiaohong Gong
> 


[prev in list] [next in list] [prev in thread] [next in thread]