RFR: 8362515: RISC-V: cleanup NativeFarCall [v3]

[Hamlin Li]

On Tue, 22 Jul 2025 15:14:22 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> I don't think this pr changes the logic in `NativeFarCall::reloc_destination`, am I right?
>> Or maybe you're misled by the name change from `stub_address` to `reloc_destination_without_check` and existing method `reloc_destination()`?
>
>> > I see `MacroAssembler::pd_call_destination` delegates work to `NativeFarCall::reloc_destination` which calls `MacroAssembler::target_addr_for_insn` under condition `if (stub_addr != nullptr)`.
>> 
>> Can you clarify "`MacroAssembler::pd_call_destination` delegates work to `NativeFarCall::reloc_destination`"? Robbin is on vacation for weeks, so I'm afraid he's not going to reponse in time.
> 
> Sorry for not being clear. Let me clarify. I mean this code snippet at [1]: 
> 
> 
>  74 address Relocation::pd_call_destination(address orig_addr) {
>  75   assert(is_call(), "should be a call here");
>  76   if (NativeCall::is_at(addr())) {
>  77     return nativeCall_at(addr())->reloc_destination();           <======================
>  78   }
>  79
>  80   if (orig_addr != nullptr) {
> 
> Before this change, we call `MacroAssembler::pd_call_destination` here.
> And `NativeFarCall::reloc_destination` at L77 will only call `MacroAssembler::target_addr_for_insn` under condition `if (stub_addr != nullptr)` [2]. But we will always/unconditionally call `MacroAssembler::target_addr_for_insn` here after this change. That seems make a difference? Did I miss anything?
> 
> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/relocInfo_riscv.cpp#L77
> [2] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/nativeInst_riscv.cpp#L112

The call path was: 
`Relocation::pd_call_destination(address orig_addr)` => 
`NativeCall::reloc_destination()` =>
`NativeFarCall::reloc_destination()`

After change, it's:
`Relocation::pd_call_destination(address orig_addr)` => 
`NativeCall::reloc_destination()` =>
`RelocCall::reloc_destination()`

The code path does not change except that `NativeFarCall::reloc_destination` is changed to `RelocCall::reloc_destination` and `assert(NativeFarCall...` is changed to `assert(RelocCall`.
Seems it does not change any logic in this code path, but just names.

> Before this change, we call MacroAssembler::pd_call_destination here.
And NativeFarCall::reloc_destination at L77 will only call MacroAssembler::target_addr_for_insn under condition if (stub_addr != nullptr)

The code between `NativeFarCall::reloc_destination` and `RelocCall::reloc_destination` is almost the same, except of names.

> But we will always/unconditionally call MacroAssembler::target_addr_for_insn here after this change.

No. I guess you might have mistoken `RelocCall::reloc_destination_without_check()` as `RelocCall::reloc_destination()`?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26370#discussion_r2224710718