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