RFR: 8332689: RISC-V: Use load instead of trampolines [v22]
Fei Yang
fyang at openjdk.org
Thu Jul 4 11:07:21 UTC 2024
On Thu, 4 Jul 2024 09:57:44 GMT, Robbin Ehn <rehn at openjdk.org> wrote:
>> 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);
Seems no need to worry about that? I think it's up to the caller to decide if it wants compressed instructions. Here in this case, the only call site is within a relocate() which will disable compressed instructions for you[1].
relocate(entry.rspec(), [&] {
load_link_jump(target);
});
[1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/assembler_riscv.hpp#L2130
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19453#discussion_r1665546545
More information about the hotspot-dev
mailing list