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

Thu Jun 23 17:57:10 UTC 2016

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 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.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 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