RFR(S): 8159620: -XX:-UseOnStackReplacement does not work together with -XX:+TieredCompilation on ppc64 and sparc

[Lindenmaier, Goetz]

Hi Volker, 

Good there was a test to bring up this additional fix!
Looks good.

Best regards,
  Goetz.

> -----Original Message-----
> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On
> Behalf Of Volker Simonis
> Sent: Mittwoch, 22. Juni 2016 16:30
> To: Tobias Hartmann <tobias.hartmann at oracle.com>
> Cc: Vladimir Kozlov <vladimir.kozlov at oracle.com>; HotSpot Open Source
> Developers <hotspot-dev at openjdk.java.net>
> Subject: Re: RFR(S): 8159620: -XX:-UseOnStackReplacement does not work
> together with -XX:+TieredCompilation on ppc64 and sparc
> 
> On Tue, Jun 21, 2016 at 10:44 AM, Tobias Hartmann
> <tobias.hartmann at oracle.com> wrote:
> > Hi Volker,
> >
> > On 21.06.2016 09:37, Volker Simonis wrote:
> >> On Mon, Jun 20, 2016 at 10:59 AM, Tobias Hartmann
> >> <tobias.hartmann at oracle.com> wrote:
> >>> Hi Volker,
> >>>
> >>> On 20.06.2016 10:52, Volker Simonis wrote:
> >>>> Hi Tobias,
> >>>>
> >>>> thanks for sponsoring! I've uploaded a new webrev with you and
> >>>> Vladimir as reviewers:
> >>>>
> >>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8159620.v2/
> >>>>
> >>>> You can find the changeset there:
> >>>>
> >>>>
> http://cr.openjdk.java.net/~simonis/webrevs/2016/8159620.v2/hotspot.cha
> ngeset
> >>>
> >>> Thanks, I just noticed that the test has
> >>>
> >>> 26  * @bug 9999999
> >>>
> >>> You can fix this in-place. I'll then run the required pre-integration testing
> (~24h) and push your change afterwards.
> >>>
> >>
> >> Hi Tobias,
> >>
> >> good catch and sorry, I somehow missed your mail yesterday.
> >>
> >> You can find the new changeset here:
> >>
> >>
> http://cr.openjdk.java.net/~simonis/webrevs/2016/8159620.v3/hotspot.cha
> ngeset
> >
> > Thank you!
> >
> > I just looked at the test results and unfortunately DisableOSRTest fails on all
> platforms (including SPARC) if -Xcomp is set:
> >
> > ----------System.err:(13/1215)----------
> > java.lang.RuntimeException: "public static void
> DisableOSRTest.main(java.lang.String[]) throws java.lang.Exception"
> shouldn't be OSR compiled if running with -XX:-UseOnStackReplacement!
> >         at DisableOSRTest.main(DisableOSRTest.java:59)
> >         at
> jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(java.base/Native
> Method)
> >         at
> jdk.internal.reflect.NativeMethodAccessorImpl.invoke(java.base/NativeMet
> hodAccessorImpl.java:62)
> >         at
> jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base/Delega
> tingMethodAccessorImpl.java:43)
> >         at java.lang.reflect.Method.invoke(java.base/Method.java:531)
> >         at
> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapp
> er.java:110)
> >         at java.lang.Thread.run(java.base/Thread.java:843)
> >
> > There are several OSR compilations in the log:
> >
> >  12916 4242 %  b  4       DisableOSRTest::main @ 19 (61 bytes)
> >
> 
> Hi Tobias,
> 
> you're right! Thanks for finding this new problem:)
> 
> I initially only fixed OSR from interpreted code. But C1 compiled code
> can also trigger an OSR request (and -Xcomp triggers a C1 compilation
> of main before it is invoked for the first time). Switching this off
> with the help of -XX:-UseOnStackReplacement never worked, but it was
> actually easy to fix. Please find the new change here:
> 
> http://cr.openjdk.java.net/~simonis/webrevs/2016/8159620.v4/
> 
> I think it would good if somebody (maybe the initial reviewers) could
> review the new C1 changes to prevent OSR in C1-compiled code.
> 
> Thank you and best regards,
> Volker
> 
> 
> > Best regards,
> > Tobias
> >
> >>
> >> Thank you and best regards,
> >> Volker
> >>
> >>
> >>> Best regards,
> >>> Tobias
> >>>
> >>>>
> >>>> Thanks,
> >>>> Volker
> >>>>
> >>>>
> >>>> On Mon, Jun 20, 2016 at 10:46 AM, Tobias Hartmann
> >>>> <tobias.hartmann at oracle.com> wrote:
> >>>>> Hi Volker,
> >>>>>
> >>>>> you fix looks good to me! I can do the sponsoring, please just send me
> a changeset.
> >>>>>
> >>>>> Best regards,
> >>>>> Tobias
> >>>>>
> >>>>> On 20.06.2016 10:16, Volker Simonis wrote:
> >>>>>> Thanks Vladimir!
> >>>>>>
> >>>>>> .. I still need a sponsor :(
> >>>>>>
> >>>>>> Regards,
> >>>>>> Volker
> >>>>>>
> >>>>>>
> >>>>>> On Fri, Jun 17, 2016 at 10:53 PM, Vladimir Kozlov
> >>>>>> <vladimir.kozlov at oracle.com> wrote:
> >>>>>>> Looks good.
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>> Vladimir
> >>>>>>>
> >>>>>>>
> >>>>>>> On 6/17/16 2:22 AM, Volker Simonis wrote:
> >>>>>>>>
> >>>>>>>> Hi Goetz,
> >>>>>>>>
> >>>>>>>> thanks for the review.
> >>>>>>>> You're right, I've fixed the "else":
> >>>>>>>>
> >>>>>>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8159620.v1/
> >>>>>>>>
> >>>>>>>> Regards,
> >>>>>>>> Volker
> >>>>>>>>
> >>>>>>>> On Fri, Jun 17, 2016 at 11:08 AM, Lindenmaier, Goetz
> >>>>>>>> <goetz.lindenmaier at sap.com> wrote:
> >>>>>>>>>
> >>>>>>>>> Hi Volker,
> >>>>>>>>>
> >>>>>>>>> thanks for doing this fix, I also have run into this issue before ...
> >>>>>>>>> Looks good.
> >>>>>>>>>
> >>>>>>>>> Small nit: usually
> >>>>>>>>>   }
> >>>>>>>>>   else {
> >>>>>>>>> are on one line.
> >>>>>>>>>
> >>>>>>>>> Best regards,
> >>>>>>>>>   Goetz.
> >>>>>>>>>
> >>>>>>>>>> -----Original Message-----
> >>>>>>>>>> From: hotspot-dev [mailto:hotspot-dev-
> bounces at openjdk.java.net] On
> >>>>>>>>>> Behalf Of Volker Simonis
> >>>>>>>>>> Sent: Donnerstag, 16. Juni 2016 16:54
> >>>>>>>>>> To: HotSpot Open Source Developers <hotspot-
> dev at openjdk.java.net>
> >>>>>>>>>> Subject: RFR(S): 8159620: -XX:-UseOnStackReplacement does
> not work
> >>>>>>>>>> together with -XX:+TieredCompilation on ppc64 and sparc
> >>>>>>>>>>
> >>>>>>>>>> Hi,
> >>>>>>>>>>
> >>>>>>>>>> can I please have a review and sponsor for the following small
> change
> >>>>>>>>>> which fixes -XX:-UseOnStackReplacement to work together
> with
> >>>>>>>>>> -XX:+TieredCompilation:
> >>>>>>>>>>
> >>>>>>>>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8159620/
> >>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8159620
> >>>>>>>>>>
> >>>>>>>>>> This is a long standing bug on SPARC and as the ppc64 template
> >>>>>>>>>> interpreter was initially forked from the SPARC
> implementation, it
> >>>>>>>>>> also manifests there. The problem is that in the case of tiered
> >>>>>>>>>> compilation the interpreter unconditionally calls
> >>>>>>>>>> InterpreterRuntime::frequency_counter_overflow if the back
> edge
> >>>>>>>>>> counter overflows. This triggers an OSR compilation, even if
> OSR was
> >>>>>>>>>> switched off with -XX:-UseOnStackReplacement.
> >>>>>>>>>>
> >>>>>>>>>> The fix is simple - just don't call
> >>>>>>>>>> InterpreterRuntime::frequency_counter_overflow if OSR has
> been
> >>>>>>>>>> switched off.
> >>>>>>>>>>
> >>>>>>>>>> Thank you and best regards,
> >>>>>>>>>> Volker