RFR: 8332689: RISC-V: Use load instead of trampolines [v4]

[Robbin Ehn]

On Wed, 5 Jun 2024 08:16:13 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> The issue is that the common code access the enum directly:
>> 
>> src/hotspot/share/code/nmethod.inline.hpp:    || (is_compiled_by_jvmci() && pc == (deopt_handler_begin() + NativeCall::instruction_size))
>> src/hotspot/share/code/nmethod.inline.hpp:    || (is_compiled_by_jvmci() && pc == (deopt_mh_handler_begin() + NativeCall::instruction_size))
>> src/hotspot/share/opto/output.cpp:  int pad_req   = NativeCall::instruction_size;
>> 
>> 
>> 
>> For c2 it just for an size estimate, so it's fine to give a larger value.
>> 
>> JVM CI is buggy (in general it's buggy all over), as we have no clue what size the unknown compiler using JVM CI generates here.
>> We can't use a constant for a C1/C2 call, so this code is just wrong.
>> Not sure what to do about, I have seen no errors for it.
>
>> The issue is that the common code access the enum directly:
>> 
>> ```
>> src/hotspot/share/code/nmethod.inline.hpp:    || (is_compiled_by_jvmci() && pc == (deopt_handler_begin() + NativeCall::instruction_size))
>> src/hotspot/share/code/nmethod.inline.hpp:    || (is_compiled_by_jvmci() && pc == (deopt_mh_handler_begin() + NativeCall::instruction_size))
>> src/hotspot/share/opto/output.cpp:  int pad_req   = NativeCall::instruction_size;
>> ```
>> 
>> For c2 it just for an size estimate, so it's fine to give a larger value.
> 
> Seems that hotspot shared code have some assumptions about the size of NativeCall. While it is OK for this c2 place, I am still a bit worried that we may have more uses of this const in hotspot shared code in future.
> 
>> JVM CI is buggy (in general it's buggy all over), as we have no clue what size the unknown compiler using JVM CI generates here. We can't use a constant for a C1/C2 call, so this code is just wrong. Not sure what to do about, I have seen no errors for it.
> 
> JVMCI is only *partially* supported on riscv for now and seems to be lack of love (See bug [1]).
> I am not sure how this change will affect them.
> 
> [1] https://bugs.openjdk.org/browse/JDK-8290154

Yea, we should probably add a `NativeCall::instruction_size()` but that limit us to have a fixed sized.
It would really be good if we could have variable size. E.g. look up size via pc desc.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19453#discussion_r1627480938