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