RFR: 8332689: RISC-V: Use load instead of trampolines [v22]
Robbin Ehn
rehn at openjdk.org
Thu Jul 4 10:00:23 UTC 2024
On Thu, 4 Jul 2024 09:22:46 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 31 commits:
>>
>> - Merge branch 'master' into 8332689
>> - Rename to reloc_call
>> - Merge branch 'master' into 8332689
>> - Rename lc
>> - Merge branch 'master' into 8332689
>> - Merge branch 'master' into 8332689
>> - Comments
>> - Missed in merge-fixes, minor revert
>> - Merge branch 'master' into 8332689
>> - Minor review comments
>> - ... and 21 more: https://git.openjdk.org/jdk/compare/38a578d5...9eabb5fa
>
> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 987:
>
>> 985: assert(is_simm32(distance), "Must be");
>> 986: Assembler::auipc(temp, (int32_t)distance + 0x800);
>> 987: Assembler::_ld(temp, temp, ((int32_t)distance << 20) >> 20);
>
> Question: Why would you use this low-level `Assembler::_ld` here?
I used to be excplicit about this is the normal **ld**.
But I see I did not do the same for **jalr** => **_jalr**. (as we are in MASM we can use 'private' method).
I'll change to normal ld and add an assert that we are in a incompressable region?
(I would like to revert that at some time, so the user of reloc_call don't need to know about it needs incompressable for reloc_call)
Suggested:
@@ -982,0 +983 @@ void MacroAssembler::load_link_jump(const address source, Register temp) {
+ assert(!in_compressible_region(), "Must be");
@@ -987 +988 @@ void MacroAssembler::load_link_jump(const address source, Register temp) {
- Assembler::_ld(temp, temp, ((int32_t)distance << 20) >> 20);
+ Assembler::ld(temp, temp, ((int32_t)distance << 20) >> 20);
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19453#discussion_r1665465771
More information about the hotspot-dev
mailing list