RFR(S): Remove some dead code in C2
Tobias Hartmann
tobias.hartmann at oracle.com
Fri Jun 19 06:06:45 UTC 2020
Pushed:
http://hg.openjdk.java.net/jdk/jdk/rev/f7587f7c859d
Best regards,
Tobias
On 18.06.20 22:27, Vladimir Kozlov wrote:
> +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