RFR(S): Remove some dead code in C2
Ludovic Henry
luhenry at microsoft.com
Wed Jun 17 05:17:47 UTC 2020
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%7Cbb17ce6d05e14499a6cf08d81235ec41%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637279370094276730&sdata=UcLt7t3CGAeysoFQS%2Bj70M6l0PQ3CG8RCpBBSyM4ssQ%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%7Cbb17ce6d05e14499a6cf08d81235ec41%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637279370094276730&sdata=zIULVvySl4sHkrzJkVpJn3lWvw2UTZsJa4rmiEHqack%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%7Cbb17ce6d05e14499a6cf08d81235ec41%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637279370094286691&sdata=oH33pbt9BMQITYtSv4fQg8qOQkFTz0MruV0%2F8bUyAqM%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%7Cbb17ce6d05e14499a6cf08d81235ec41%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637279370094286691&sdata=WHWgZyG3CDhIErnYcnIg9PHgQ9GiWOx4bCtZN55MnvQ%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