RFR: 8337987: Relocate jfr and throw_exception stubs from StubGenerator to SharedRuntime [v4]

[Andrew Dinn]

On Wed, 14 Aug 2024 10:00:19 GMT, Yudi Zheng <yzheng at openjdk.org> wrote:

>> Andrew Dinn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix riscv port issues
>
> src/hotspot/share/jvmci/vmStructs_jvmci.cpp line 330:
> 
>> 328:   static_field(StubRoutines,                _verify_oop_count,                                jint)                                  \
>> 329:                                                                                                                                      \
>> 330:   static_field(StubRoutines,                _throw_delayed_StackOverflowError_entry,          address)                               \
> 
> Please add the following symbol exporting
> 
> diff --git a/src/hotspot/share/jvmci/jvmciCompilerToVM.hpp b/src/hotspot/share/jvmci/jvmciCompilerToVM.hpp
> index 8fdb96a3038..0e96cea6596 100644
> --- a/src/hotspot/share/jvmci/jvmciCompilerToVM.hpp
> +++ b/src/hotspot/share/jvmci/jvmciCompilerToVM.hpp
> @@ -50,6 +50,7 @@ class CompilerToVM {
>      static address SharedRuntime_deopt_blob_unpack_with_exception_in_tls;
>      static address SharedRuntime_deopt_blob_uncommon_trap;
>      static address SharedRuntime_polling_page_return_handler;
> +    static address SharedRuntime_throw_delayed_StackOverflowError_blob;
>  
>      static address nmethod_entry_barrier;
>      static int thread_disarmed_guard_value_offset;
> diff --git a/src/hotspot/share/jvmci/jvmciCompilerToVMInit.cpp b/src/hotspot/share/jvmci/jvmciCompilerToVMInit.cpp
> index 2116133e56e..27031bf55fe 100644
> --- a/src/hotspot/share/jvmci/jvmciCompilerToVMInit.cpp
> +++ b/src/hotspot/share/jvmci/jvmciCompilerToVMInit.cpp
> @@ -68,6 +68,7 @@ address CompilerToVM::Data::SharedRuntime_deopt_blob_unpack;
>  address CompilerToVM::Data::SharedRuntime_deopt_blob_unpack_with_exception_in_tls;
>  address CompilerToVM::Data::SharedRuntime_deopt_blob_uncommon_trap;
>  address CompilerToVM::Data::SharedRuntime_polling_page_return_handler;
> +address CompilerToVM::Data::SharedRuntime_throw_delayed_StackOverflowError_blob;
>  
>  address CompilerToVM::Data::nmethod_entry_barrier;
>  int CompilerToVM::Data::thread_disarmed_guard_value_offset;
> @@ -158,6 +159,7 @@ void CompilerToVM::Data::initialize(JVMCI_TRAPS) {
>    SharedRuntime_deopt_blob_unpack_with_exception_in_tls = SharedRuntime::deopt_blob()->unpack_with_exception_in_tls();
>    SharedRuntime_deopt_blob_uncommon_trap = SharedRuntime::deopt_blob()->uncommon_trap();
>    SharedRuntime_polling_page_return_handler = SharedRuntime::polling_page_return_handler_blob()->entry_point();
> +  SharedRuntime_throw_delayed_StackOverflowError_blob = SharedRuntime::throw_delayed_StackOverflowError_entry();
>  
>    BarrierSetNMethod* bs_nm = BarrierSet::b...

Hi @mur47x111. Thanks for looking at this PR.

I deleted the static field declaration from `vmStructs_jvmci.cpp` because I found no other mention of `throw_delayed_StackOverflowError_entry` under `src/hotspot/share/jvmci`. So, I assumed it was not being used by any JVMCI clients.

I am happy to push a patch with your proposed changes. However, I just wanted to check whether you are sure you want to use the name `SharedRuntime_throw_delayed_StackOverflowError_blob` in the new declarations of the `CompilerToVM` field.

I am asking because I when I moved the exception stubs from `StubGenerator` to `SharedRuntime` I also made a change in the way the stubs are named, stored and accessed.

The original field `StubGenerator::SharedRuntime_throw_delayed_StackOverflowError_entry` stored the entry point for the throw routine and was of type `address`. The getter method that returns the entry address simply returns the field value.

The relocated field `SharedRuntime::_throw_delayed_StackOverflowError_blob` stores a pointer to a `RuntimeBlob`, the one that contains the throw routine. That is why the field name ends with `_blob` not `_entry`. The entry address is not stored directly in class `SharedRuntime`. In the new code the getter method computes the address by calling the blob's `entry_point()` method.

So, your patch is seems to be mixing up things by using the name `SharedRuntime_throw_delayed_StackOverflowError_blob` for the `CompilerToVM` field but declaring it with type `address`. Would it make more sense to use the name `SharedRuntime_throw_delayed_StackOverflowError_entry` for this field?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20566#discussion_r1716914885