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

Fri Jun 24 09:17:10 UTC 2016

Thanks Vladimir.

Best regards,
Volker

On Thu, Jun 23, 2016 at 7:57 PM, Vladimir Kozlov
<vladimir.kozlov at 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 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