RFR: 8234906: [TESTBUG] TestDivZeroCheckControl fails for client VMs due to Unrecognized VM option LoopUnrollLimit

[christoph.goettschkes at microdoc.com]

Hi Vladimir,

thanks for the explanation and for pointing to the code.

I integrated your suggestions into this new webrev:
https://cr.openjdk.java.net/~cgo/8234906/webrev.02/

I already created the changeset with you as a reviewer, so if you are fine 
with this version, may I ask you to sponsor it and commit it into the 
repository for me?
https://cr.openjdk.java.net/~cgo/8234906/webrev.02/jdk-jdk.changeset

Thanks,
Christoph

Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote on 2019-12-03 19:33:52:

> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
> To: christoph.goettschkes at microdoc.com
> Cc: hotspot-compiler-dev at openjdk.java.net
> Date: 2019-12-03 19:34
> Subject: Re: RFR: 8234906: [TESTBUG] TestDivZeroCheckControl fails for 
> client VMs due to Unrecognized VM option LoopUnrollLimit
> 
> You don't need to duplicate @bug (it was C2 bug anyway). And don't need
> to check  & !vm.graal.enabled for C2 case (see 
> explanation below)
> 
>  > Could you elaborate how the two flags are related? I though, if graal 
is
>  > used as a JIT, both `vm.graal.enabled` and `vm.compiler2.enabled` are 
set
>  > to true. Is that correct? I don't have a setup with graal, so I can 
not
>  > test this.
> 
> When we enable Graal JIT it is used instead of C2. They are mutually 
exclusive.
> Compiler.isC2Enabled() (which sets vm.compiler2.enabled [1]) returns 
> false when isGraalEnabled() returns true [2].
> 
> Regards,
> Vladimir
> 
> [1] http://hg.openjdk.java.net/jdk/jdk/file/138b0f3fe18c/test/jtreg-
> ext/requires/VMProps.java#l117
> [2] http://hg.openjdk.java.net/jdk/jdk/file/138b0f3fe18c/test/lib/sun/
> hotspot/code/Compiler.java#l80
> 
> 
> On 12/3/19 4:22 AM, christoph.goettschkes at microdoc.com wrote:
> > Hi Vladimir,
> > 
> > could you have a look at my updated webrev regarding this failing 
test?
> > https://cr.openjdk.java.net/~cgo/8234906/webrev.01/
> > 
> > See my inline comments in the mail below.
> > 
> > Thanks,
> > Christpoh
> > 
> > "hotspot-compiler-dev" <hotspot-compiler-dev-bounces at openjdk.java.net>
> > wrote on 2019-11-28 13:44:05:
> > 
> >> From: christoph.goettschkes at microdoc.com
> >> To: Vladimir Kozlov <vladimir.kozlov at oracle.com>
> >> Cc: hotspot-compiler-dev at openjdk.java.net
> >> Date: 2019-11-28 13:46
> >> Subject: Re: RFR: 8234906: [TESTBUG] TestDivZeroCheckControl fails 
for
> >> client VMs due to Unrecognized VM option LoopUnrollLimit
> >> Sent by: "hotspot-compiler-dev"
> > <hotspot-compiler-dev-bounces at openjdk.java.net>
> >>
> >> Hi Vladimir,
> >>
> >> Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote on 2019-11-27
> > 20:54:02:
> >>
> >>> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
> >>> To: christoph.goettschkes at microdoc.com,
> >> hotspot-compiler-dev at openjdk.java.net
> >>> Date: 2019-11-27 20:54
> >>> Subject: Re: RFR: 8234906: [TESTBUG] TestDivZeroCheckControl fails 
for
> > 
> >>> client VMs due to Unrecognized VM option LoopUnrollLimit
> >>>
> >>> Hi Christoph
> >>>
> >>> I was about suggest IgnoreUnrecognizedVMOptions flag but remembered
> >>> discussion about 8231954 fix.
> >>
> >> Yes, I try to avoid "IgnoreUnrecognizedVMOptions" because of our
> > previous
> >> discussion. I also think that it doesn't make sense to execute tests 
in
> > VM
> >> configurations for which they are not written for. Most of the 
compiler
> >> tests simply have "IgnoreUnrecognizedVMOptions" and probably waste a
> > good
> >> amount of time in certain VM configurations.
> >>
> >>> But I think the test should be run with Graal - it does have OSR
> >>> compilation and we need to test it.
> >>
> >> Sure. I disabled it, because I thought that the flag 
"LoopUnrollLimit"
> > is
> >> required to trigger the faulty behavior, but I don't know much about
> >> optimization in the graal JIT.
> >>
> >>>
> >>> We can do it by splitting test runs (duplicate @test block with
> >>> different run flags) to have 2 tests with different
> >>> flags and conditions. See [1].
> >>>
> >>> For existing @run block we use `@requires vm.compiler2.enabled` and
> > for
> >>> new without LoopUnrollLimit - `vm.graal.enabled`.
> >>
> >> I did the following:
> >>
> >> https://cr.openjdk.java.net/~cgo/8234906/webrev.01/
> >>
> >> Could you elaborate how the two flags are related? I though, if graal 
is
> > 
> >> used as a JIT, both `vm.graal.enabled` and `vm.compiler2.enabled` are
> > set
> >> to true. Is that correct? I don't have a setup with graal, so I can 
not
> >> test this.
> >>
> >> Thanks,
> >> Christoph
> >>
> > 
>