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