RFR: 8326306: RISC-V: Re-structure MASM calls and jumps [v2]

Robbin Ehn rehn at openjdk.org
Fri Apr 26 09:28:57 UTC 2024


On Fri, 26 Apr 2024 07:21:48 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> src/hotspot/cpu/riscv/gc/shenandoah/shenandoahBarrierSetAssembler_riscv.cpp line 303:
>> 
>>> 301:     target = CAST_FROM_FN_PTR(address, ShenandoahRuntime::load_reference_barrier_weak);
>>> 302:   }
>>> 303:   __ rt_call(target);
>> 
>> Question: does it make sense to replace `call` with `rt_call` when we are invoking the VM code (C++ code)? Here is what I see the difference between the two: `rt_call` emits code (`auipc` or `movptr`) depending on whether the destination could be found in code cache, while `call` depends on `is_32bit_offset_from_codeache`. So it's still possible for `call` to emit the short `auipc` code if not far even when the target is not there in the code cache like this case. But `rt_call` will always emit a long `movptr` sequence for this case, which I think is not good in performance.
>
> A couple of point, all calls to VM runtime should use "call_VM_leaf".
> E.g.
> `  __ call_VM_leaf(Continuation::freeze_entry(), 2);`
> AFIACT it is only Shenandoah which calls VM is this 'wrong' way.
> 
> call_VM_leaf always use mv -> li.
> 
> - It would be much better to change call_VM_leaf to use auipc is possible. (and fix Shenandoah to use call_VM_leaf)
> - We can probably remove rt_call, and just have call.

I found some other places which uses plain calls to leaf, instead of call_VM_leaf.
It seems like it's a guess that registers don't need push/pop, I don't think such speculation is good.
If it must be there we should have a argument to call_VM_leaf saying we don't want to push/pop.

x86 is this case uses the correct:
` __ call_VM_leaf(CAST_FROM_FN_PTR(address, ShenandoahRuntime::load_reference_barrier_weak), c_rarg0, c_rarg1);`

rt_call for non-code-cache do not need relocation, i.e. movptr.
So rt_call and call is not simply interchangeable, i.e. you now need relocation (pc relative calls). 

Yes, we can probably do better here, but as the change is mv/li + jalr to movptr + jalr, there is no regression.
So improvement should be done outside of this PR.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18942#discussion_r1580743377


More information about the hotspot-dev mailing list