RFR: 8332689: RISC-V: Use load instead of trampolines [v11]
Robbin Ehn
rehn at openjdk.org
Fri Jun 14 12:54:23 UTC 2024
On Fri, 14 Jun 2024 06:58:14 GMT, Fei Yang <fyang at openjdk.org> wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 16 commits:
>>
>> - Merge branch 'master' into 8332689
>> - Merge branch 'master' into 8332689
>> - Merge branch 'master' into 8332689
>> - Merge branch 'master' into 8332689
>> - Remove tmp file
>> - Prepare for dynamic NativeCall size
>> - Only allow one calling convetion, i.e. fixed sized
>> - Merge branch 'master' into 8332689
>> - Review comments
>> - Move shart/far code to cpp
>> - ... and 6 more: https://git.openjdk.org/jdk/compare/5d2a19de...bb7249b8
>
> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 3732:
>
>> 3730: // Maybe emit a call via a trampoline. If the code cache is small
>> 3731: // trampolines won't be emitted.
>> 3732: address MacroAssembler::patchable_far_call(Address entry) {
>
> It doesn't look nice to me for `UseTrampolines` checks to be spread across this `MacroAssembler::patchable_far_call` function. I would suggest to keep the original `MacroAssembler::trampoline_call` and let `MacroAssembler::patchable_far_call` delegate work to it under `UseTrampolines`. What do you think?
fixed
> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 4642:
>
>> 4640: }
>> 4641: } else {
>> 4642: rt_call(zero_blocks.target(), t0);
>
> Maybe simply: `rt_call(zero_blocks.target());` as `t0` is the default temp register for `rt_call`.
ok
> src/hotspot/cpu/riscv/macroAssembler_riscv.hpp line 1201:
>
>> 1199: //
>> 1200: // Old patchable far calls: (-XX:+UseTrampolines)
>> 1201: // - trampoline call:
>
> How about combine the two lines? Like:
> `- trampoline call (old patchable far call / -XX:+UseTrampolines):`
ok
> src/hotspot/cpu/riscv/macroAssembler_riscv.hpp line 1240:
>
>> 1238:
>> 1239: // Emit a direct call if the entry address will always be in range,
>> 1240: // otherwise a patachable far call.
>
> s/patachable/patchable/
fixed
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19453#discussion_r1639778686
PR Review Comment: https://git.openjdk.org/jdk/pull/19453#discussion_r1639778123
PR Review Comment: https://git.openjdk.org/jdk/pull/19453#discussion_r1639776855
PR Review Comment: https://git.openjdk.org/jdk/pull/19453#discussion_r1639776675
More information about the hotspot-dev
mailing list