[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-dev
Subject: Re: RFR(S): 8159620: -XX:-UseOnStackReplacement does not work together with -XX:+TieredCompilation o
From: Volker Simonis <volker.simonis () gmail ! com>
Date: 2016-06-24 9:17:10
Message-ID: CA+3eh132-tNYdKgZiub9iF2umbL1_U+gxeeJGzQ=YPVZSnb_Hw () mail ! gmail ! com
[Download RAW message or body]
Thanks Vladimir.
Best regards,
Volker
On Thu, Jun 23, 2016 at 7:57 PM, Vladimir Kozlov
<vladimir.kozlov@oracle.com> wrote:
> Reviewed.
>
> PPC changes looks fine.
> SPARC changes are good - I was worried about annulling in branch in
> increment_mask_and_jump() but it is fine since it is false.
> C1 - new check is correct.
>
> Test is fine.
>
> Thanks,
> Vladimir
>
>
> On 6/22/16 7:29 AM, Volker Simonis wrote:
> >
> > On Tue, Jun 21, 2016 at 10:44 AM, Tobias Hartmann
> > <tobias.hartmann@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@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.changeset
> > > > > >
> > > > >
> > > > >
> > > > > 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.changeset
> > >
> > >
> > > 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/NativeMethodAccessorImpl.java:62)
> > > at
> > > jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base/DelegatingMethodAccessorImpl.java:43)
> > > at java.lang.reflect.Method.invoke(java.base/Method.java:531)
> > > at
> > > com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.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@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@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@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@openjdk.java.net] On
> > > > > > > > > > > > Behalf Of Volker Simonis
> > > > > > > > > > > > Sent: Donnerstag, 16. Juni 2016 16:54
> > > > > > > > > > > > To: HotSpot Open Source Developers
> > > > > > > > > > > > <hotspot-dev@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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic