RFR: JDK-8332423 : [PPC64] Remove C1_MacroAssembler::call_c_with_frame_resize [v2]

[Martin Doerr]

On Sun, 25 Aug 2024 17:00:33 GMT, Suchismith Roy <sroy at openjdk.org> wrote:

>> JBS Issue : [JDK-8332423](https://bugs.openjdk.org/browse/JDK-8332423)
>> C1_MacroAssembler::call_c_with_frame_resize is only used with frame_resize == 0. 
>> Also, call_c is adapted as per endianess of system. 
>> We can adapt the exisiting code to handle the endianness check at one place and not have to repeatedly check at multiple places to make calls to call_c.
>
> Suchismith Roy has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove this->

Nice cleanup! I have a couple of small change requests.

src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp line 138:

> 136:     // func to get the address of the same-named entrypoint in the
> 137:     // generated interpreter code.
> 138:     call_c(CAST_FROM_FN_PTR(address, Interpreter::remove_activation_preserving_args_entry), relocInfo::none);

`relocInfo::none` can be omitted. It's the default.

src/hotspot/cpu/ppc/macroAssembler_ppc.cpp line 1038:

> 1036: }
> 1037: 
> 1038: address MacroAssembler::call_c(address function_entry,relocInfo::relocType) {

Please restore the original line.

src/hotspot/cpu/ppc/macroAssembler_ppc.hpp line 367:

> 365:   // calling conventions. Updates and returns _last_calls_return_pc.
> 366:   address call_c(Register function_descriptor);
> 367:   address call_c(address function_entry, relocInfo::relocType rt = relocInfo::relocType::none) {

I think `relocInfo::none` should be used like above.

src/hotspot/cpu/ppc/macroAssembler_ppc.hpp line 419:

> 417:   void call_VM_leaf(address entry_point, Register arg_1, Register arg_2);
> 418:   void call_VM_leaf(address entry_point, Register arg_1, Register arg_2, Register arg_3);
> 419: 

Please don't remove the empty line.

src/hotspot/cpu/ppc/stubGenerator_ppc.cpp line 619:

> 617:     return stub->entry_point();
> 618:   }
> 619: #undef __

I guess this was added unintentionally?

src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp line 1467:

> 1465: 
> 1466:   __ mr(R3_ARG1, R16_thread);
> 1467:   __ call_c(CAST_FROM_FN_PTR(address, JavaThread::check_special_condition_for_native_trans), relocInfo::none);

`relocInfo::none` can be omitted. It's the default.

-------------

Changes requested by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19947#pullrequestreview-2264025594
PR Review Comment: https://git.openjdk.org/jdk/pull/19947#discussion_r1733252476
PR Review Comment: https://git.openjdk.org/jdk/pull/19947#discussion_r1733248903
PR Review Comment: https://git.openjdk.org/jdk/pull/19947#discussion_r1733249839
PR Review Comment: https://git.openjdk.org/jdk/pull/19947#discussion_r1733250155
PR Review Comment: https://git.openjdk.org/jdk/pull/19947#discussion_r1733253411
PR Review Comment: https://git.openjdk.org/jdk/pull/19947#discussion_r1733253817