RFR(S): Remove some dead code in C2
Vladimir Kozlov
vladimir.kozlov at oracle.com
Thu Jun 18 20:27:51 UTC 2020
+1
Thanks,
Vladimir K
On 6/18/20 4:43 AM, Tobias Hartmann wrote:
> Hi Ludovic,
>
> thanks for updating. Looks good to me.
>
> Best regards,
> Tobias
>
> On 17.06.20 21:39, Ludovic Henry wrote:
>> The webrev is available at http://cr.openjdk.java.net/~adityam/ludovic/rem_delayed_forbidden/jdk.patch. It also contains the fix for https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-June/038636.html.
>>
>> --
>> Ludovic
>>
>> -----Original Message-----
>> From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> On Behalf Of Ludovic Henry
>> Sent: Tuesday, June 16, 2020 10:18 PM
>> To: Vladimir Ivanov <vladimir.x.ivanov at oracle.com>; Vladimir Kozlov <vladimir.kozlov at oracle.com>; hotspot-compiler-dev at openjdk.java.net
>> Subject: RE: RFR(S): Remove some dead code in C2
>>
>> Hi,
>>
>> Thank you for the review!
>>
>> I do not have authorship, so feel free to take my change and commit them directly (if that's the appropriate thing to do of course!). I'll work with a colleague who has authorship to get a webrev going, but feel free to take it from there if you want to see it happen sooner rather than later.
>>
>> --
>> Ludovic
>>
>> -----Original Message-----
>> From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> On Behalf Of Vladimir Ivanov
>> Sent: Tuesday, June 16, 2020 11:15 AM
>> To: Vladimir Kozlov <vladimir.kozlov at oracle.com>; hotspot-compiler-dev at openjdk.java.net
>> Subject: Re: RFR(S): Remove some dead code in C2
>>
>> Good catch, Ludovic!
>>
>> Looks good to me as well.
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> On 16.06.2020 19:38, Vladimir Kozlov wrote:
>>> Hi Ludovic,
>>>
>>> Looks good.
>>>
>>> Yes, in 8148994 Vladimir Ivanov enable late inlining [1]:
>>>
>>> "But after JDK-8072008 there's no problem with delaying inlining. C2 can
>>> decide whether to keep the direct call or inline through it. So, I
>>> enabled late inlining for all linkers. (Surprisingly, no significant
>>> performance difference on nashorn.)"
>>>
>>> I created RFE https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8247697&data=02%7C01%7Cluhenry%40microsoft.com%7Cd39a7504cd9c40d117c508d8127e0f37%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637279679919387791&sdata=gwXvpyGBz7IE4Vq7Kr95v5KRysoGKEEpg4tXil3y%2FSg%3D&reserved=0 for this.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> [1]
>>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openjdk.java.net%2Fpipermail%2Fhotspot-compiler-dev%2F2016-February%2F021137.html&data=02%7C01%7Cluhenry%40microsoft.com%7Cd39a7504cd9c40d117c508d8127e0f37%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637279679919387791&sdata=mgOIg%2F9zsu5f22BDoeKRnZsUX2VE7WmQtpHKChbSRMU%3D&reserved=0
>>>
>>>
>>> PS: Does someone in your group have OpenJDK Author status so he can file
>>> issues in JBS?
>>>
>>> On 6/15/20 4:58 PM, Ludovic Henry wrote:
>>>> Hi,
>>>>
>>>> As I was exploring code in src/hotspot/share/opto/doCall.cpp, I
>>>> noticed the `delayed_forbidden` parameter to `Compile::call_generator`
>>>> never to be used.
>>>>
>>>> From doing some mercurial archeology, the change was introduced with
>>>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fhg.openjdk.java.net%2Fjdk%2Fjdk%2Frev%2F823590505eb4&data=02%7C01%7Cluhenry%40microsoft.com%7Cd39a7504cd9c40d117c508d8127e0f37%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637279679919387791&sdata=oAeWirekxu3yo5CSiSND1lYSvRnc8pw5blBYFlXegqY%3D&reserved=0, and later
>>>> modified with https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fhg.openjdk.java.net%2Fjdk%2Fjdk%2Frev%2Fe1685e30beca&data=02%7C01%7Cluhenry%40microsoft.com%7Cd39a7504cd9c40d117c508d8127e0f37%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637279679919387791&sdata=TsPQvy9ZIoNPMZaazl8nA%2F9F0iPQpghHMRRp0QbT3Ec%3D&reserved=0
>>>> which removed all uses of `delayed_forbidden`.
>>>>
>>>> As I am not an author, please find following the patch removing the
>>>> dead code.
>>>>
>>>> Thank you,
>>>>
>>>> --
>>>> Ludovic
>>>>
>>>> diff --git a/src/hotspot/share/opto/callGenerator.cpp
>>>> b/src/hotspot/share/opto/callGenerator.cpp
>>>> index 1092d582184..606325d4dd6 100644
>>>> --- a/src/hotspot/share/opto/callGenerator.cpp
>>>> +++ b/src/hotspot/share/opto/callGenerator.cpp
>>>> @@ -821,13 +821,13 @@ JVMState*
>>>> PredictedCallGenerator::generate(JVMState* jvms) {
>>>> }
>>>>
>>>>
>>>> -CallGenerator* CallGenerator::for_method_handle_call(JVMState* jvms,
>>>> ciMethod* caller, ciMethod* callee, bool delayed_forbidden) {
>>>> +CallGenerator* CallGenerator::for_method_handle_call(JVMState* jvms,
>>>> ciMethod* caller, ciMethod* callee) {
>>>> assert(callee->is_method_handle_intrinsic(),
>>>> "for_method_handle_call mismatch");
>>>> bool input_not_const;
>>>> CallGenerator* cg = CallGenerator::for_method_handle_inline(jvms,
>>>> caller, callee, input_not_const);
>>>> Compile* C = Compile::current();
>>>> if (cg != NULL) {
>>>> - if (!delayed_forbidden && AlwaysIncrementalInline) {
>>>> + if (AlwaysIncrementalInline) {
>>>> return CallGenerator::for_late_inline(callee, cg);
>>>> } else {
>>>> return cg;
>>>> diff --git a/src/hotspot/share/opto/callGenerator.hpp
>>>> b/src/hotspot/share/opto/callGenerator.hpp
>>>> index 46bf9f5d7f9..3a04fa4d5cd 100644
>>>> --- a/src/hotspot/share/opto/callGenerator.hpp
>>>> +++ b/src/hotspot/share/opto/callGenerator.hpp
>>>> @@ -124,7 +124,7 @@ class CallGenerator : public ResourceObj {
>>>> static CallGenerator* for_direct_call(ciMethod* m, bool
>>>> separate_io_projs = false); // static, special
>>>> static CallGenerator* for_virtual_call(ciMethod* m, int
>>>> vtable_index); // virtual, interface
>>>>
>>>> - static CallGenerator* for_method_handle_call( JVMState* jvms,
>>>> ciMethod* caller, ciMethod* callee, bool delayed_forbidden);
>>>> + static CallGenerator* for_method_handle_call( JVMState* jvms,
>>>> ciMethod* caller, ciMethod* callee);
>>>> static CallGenerator* for_method_handle_inline(JVMState* jvms,
>>>> ciMethod* caller, ciMethod* callee, bool& input_not_const);
>>>>
>>>> // How to generate a replace a direct call with an inline version
>>>> diff --git a/src/hotspot/share/opto/compile.hpp
>>>> b/src/hotspot/share/opto/compile.hpp
>>>> index a922a905707..bd0def2997c 100644
>>>> --- a/src/hotspot/share/opto/compile.hpp
>>>> +++ b/src/hotspot/share/opto/compile.hpp
>>>> @@ -854,7 +854,7 @@ class Compile : public Phase {
>>>> // The profile factor is a discount to apply to this site's
>>>> interp. profile.
>>>> CallGenerator* call_generator(ciMethod* call_method, int
>>>> vtable_index, bool call_does_dispatch,
>>>> JVMState* jvms, bool
>>>> allow_inline, float profile_factor, ciKlass* speculative_receiver_type
>>>> = NULL,
>>>> - bool allow_intrinsics = true, bool
>>>> delayed_forbidden = false);
>>>> + bool allow_intrinsics = true);
>>>> bool should_delay_inlining(ciMethod* call_method, JVMState* jvms) {
>>>> return should_delay_string_inlining(call_method, jvms) ||
>>>> should_delay_boxing_inlining(call_method, jvms);
>>>> diff --git a/src/hotspot/share/opto/doCall.cpp
>>>> b/src/hotspot/share/opto/doCall.cpp
>>>> index c26dc4b682d..c4d55d0d4c4 100644
>>>> --- a/src/hotspot/share/opto/doCall.cpp
>>>> +++ b/src/hotspot/share/opto/doCall.cpp
>>>> @@ -65,7 +65,7 @@ void trace_type_profile(Compile* C, ciMethod
>>>> *method, int depth, int bci, ciMeth
>>>> CallGenerator* Compile::call_generator(ciMethod* callee, int
>>>> vtable_index, bool call_does_dispatch,
>>>> JVMState* jvms, bool
>>>> allow_inline,
>>>> float prof_factor, ciKlass*
>>>> speculative_receiver_type,
>>>> - bool allow_intrinsics, bool
>>>> delayed_forbidden) {
>>>> + bool allow_intrinsics) {
>>>> ciMethod* caller = jvms->method();
>>>> int bci = jvms->bci();
>>>> Bytecodes::Code bytecode = caller->java_code_at_bci(bci);
>>>> @@ -145,8 +145,8 @@ CallGenerator* Compile::call_generator(ciMethod*
>>>> callee, int vtable_index, bool
>>>> // MethodHandle.invoke* are native methods which obviously don't
>>>> // have bytecodes and so normal inlining fails.
>>>> if (callee->is_method_handle_intrinsic()) {
>>>> - CallGenerator* cg = CallGenerator::for_method_handle_call(jvms,
>>>> caller, callee, delayed_forbidden);
>>>> - assert(cg == NULL || !delayed_forbidden || !cg->is_late_inline()
>>>> || cg->is_mh_late_inline(), "unexpected CallGenerator");
>>>> + CallGenerator* cg = CallGenerator::for_method_handle_call(jvms,
>>>> caller, callee);
>>>> + assert(cg == NULL || !cg->is_late_inline() ||
>>>> cg->is_mh_late_inline(), "unexpected CallGenerator");
>>>> return cg;
>>>> }
>>>>
>>>> @@ -182,12 +182,10 @@ CallGenerator* Compile::call_generator(ciMethod*
>>>> callee, int vtable_index, bool
>>>> // opportunity to perform some high level optimizations
>>>> // first.
>>>> if (should_delay_string_inlining(callee, jvms)) {
>>>> - assert(!delayed_forbidden, "strange");
>>>> return CallGenerator::for_string_late_inline(callee, cg);
>>>> } else if (should_delay_boxing_inlining(callee, jvms)) {
>>>> - assert(!delayed_forbidden, "strange");
>>>> return CallGenerator::for_boxing_late_inline(callee, cg);
>>>> - } else if ((should_delay || AlwaysIncrementalInline) &&
>>>> !delayed_forbidden) {
>>>> + } else if ((should_delay || AlwaysIncrementalInline)) {
>>>> return CallGenerator::for_late_inline(callee, cg);
>>>> }
>>>> }
>>>>
>>>>
More information about the hotspot-compiler-dev
mailing list