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

[Yudi Zheng]

On Wed, 14 Aug 2024 13:19:35 GMT, Andrew Dinn <adinn at openjdk.org> wrote:

>> 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_St...
>
> 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?

Right, `SharedRuntime_throw_delayed_StackOverflowError_entry` is better. Thanks for the explanation!

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

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