RFR(S): Remove some dead code in C2
Tobias Hartmann
tobias.hartmann at oracle.com
Thu Jun 18 11:43:35 UTC 2020
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