RFR: 8369296: Add fast class init checks in interpreter for resolving ConstantPool entries for static field [v4]

[Martin Doerr]

On Thu, 9 Oct 2025 08:35:05 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> Thanks for the ping! PPC64 implementation:
>> 
>> diff --git a/src/hotspot/cpu/ppc/templateTable_ppc_64.cpp b/src/hotspot/cpu/ppc/templateTable_ppc_64.cpp
>> index 7431f77aeff..373dc24a62f 100644
>> --- a/src/hotspot/cpu/ppc/templateTable_ppc_64.cpp
>> +++ b/src/hotspot/cpu/ppc/templateTable_ppc_64.cpp
>> @@ -2179,17 +2179,11 @@ void TemplateTable::_return(TosState state) {
>>  //   - Rscratch
>>  void TemplateTable::resolve_cache_and_index_for_method(int byte_no, Register Rcache, Register Rscratch) {
>>    assert(byte_no == f1_byte || byte_no == f2_byte, "byte_no out of range");
>> +
>>    Label Lresolved, Ldone, L_clinit_barrier_slow;
>>    Register Rindex = Rscratch;
>>  
>>    Bytecodes::Code code = bytecode();
>> -  switch (code) {
>> -    case Bytecodes::_nofast_getfield: code = Bytecodes::_getfield; break;
>> -    case Bytecodes::_nofast_putfield: code = Bytecodes::_putfield; break;
>> -    default:
>> -      break;
>> -  }
>> -
>>    const int bytecode_offset = (byte_no == f1_byte) ? in_bytes(ResolvedMethodEntry::bytecode1_offset())
>>                                                     : in_bytes(ResolvedMethodEntry::bytecode2_offset());
>>    __ load_method_entry(Rcache, Rindex);
>> @@ -2201,10 +2195,9 @@ void TemplateTable::resolve_cache_and_index_for_method(int byte_no, Register Rca
>>  
>>    // Class initialization barrier slow path lands here as well.
>>    __ bind(L_clinit_barrier_slow);
>> -
>>    address entry = CAST_FROM_FN_PTR(address, InterpreterRuntime::resolve_from_cache);
>>    __ li(R4_ARG2, code);
>> -  __ call_VM(noreg, entry, R4_ARG2, true);
>> +  __ call_VM(noreg, entry, R4_ARG2);
>>  
>>    // Update registers with resolved info.
>>    __ load_method_entry(Rcache, Rindex);
>> @@ -2226,12 +2219,10 @@ void TemplateTable::resolve_cache_and_index_for_method(int byte_no, Register Rca
>>    __ bind(Ldone);
>>  }
>>  
>> -void TemplateTable::resolve_cache_and_index_for_field(int byte_no,
>> -                                            Register Rcache,
>> -                                            Register index) {
>> +void TemplateTable::resolve_cache_and_index_for_field(int byte_no, Register Rcache, Register index) {
>>    assert_different_registers(Rcache, index);
>>  
>> -  Label resolved;
>> +  Label Lresolved, Ldone, L_clinit_barrier_slow;
>>  
>>    Bytecodes::Code code = bytecode();
>>    switch (code) {
>> @@ -2246,19 +2237,34 @@ void TemplateTable::resolve_cache_and_index_for_field(int byte_no,
>>                                           : in_bytes(ResolvedFieldEntry::put...
>
>> @TheRealMDoerr with your patch `isync` is bypassed after `InterpreterRuntime::resolve_from_cache` call. Is it intentional?
> 
> Thanks for pointing this out. It is ok because the same thread has performed the resolution. So, no need for a memory barrier in this case because we didn't read values written by another thread.
> And even if another thread has done it, we already have enough memory barriers because `resolve_from_cache` contains a thread state switch (at least for PPC64 which uses lwsync).

> @TheRealMDoerr thanks for the patch. In my last commit I have updated the code to move the fast clinit check before the call to `resolve_from_cache`. I felt this simplifies the flow of logic and makes it easier to read. @iwanowww what do you think about this?

That looks better. However, seems like for forgot a small detail (PPC64 example):

diff --git a/src/hotspot/cpu/ppc/templateTable_ppc_64.cpp b/src/hotspot/cpu/ppc/templateTable_ppc_64.cpp
index 14fdbfe0884..8657356de4a 100644
--- a/src/hotspot/cpu/ppc/templateTable_ppc_64.cpp
+++ b/src/hotspot/cpu/ppc/templateTable_ppc_64.cpp
@@ -2203,6 +2203,8 @@ void TemplateTable::resolve_cache_and_index_for_method(int byte_no, Register Rca
     __ ld(method, in_bytes(ResolvedMethodEntry::method_offset()), Rcache);
     __ load_method_holder(klass, method);
     __ clinit_barrier(klass, R16_thread, &Ldone, /*L_slow_path*/ nullptr);
+  } else {
+    __ b(Ldone);
   }
 
   // Class initialization barrier slow path lands here as well.
@@ -2247,6 +2249,8 @@ void TemplateTable::resolve_cache_and_index_for_field(int byte_no, Register Rcac
     // (ordered by compare-branch-isync).
     __ ld(field_holder, ResolvedFieldEntry::field_holder_offset(), Rcache);
     __ clinit_barrier(field_holder, R16_thread, &Ldone, /*L_slow_path*/ nullptr);
+  } else {
+    __ b(Ldone);
   }
 
   // resolve first time through

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

PR Comment: https://git.openjdk.org/jdk/pull/27676#issuecomment-3386202228