RFR: 8337396: Cleanup usage of ExternalAddess [v3]

[Fei Yang]

On Thu, 1 Aug 2024 18:38:47 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> `ExternalAddess` should be used only for data load. For calls (and jump) instructions we should use `RuntimeAddress` which uses `runtime_call_Relocation`.
>> 
>> I found few places where `ExternalAddess` is used incorrectly and fixed them.
>> 
>> I also added code to print "hottest" (most referenced) `ExternalAddess` addresses in global table to move them into static global tables which will be introduced by [JDK-8334691](https://bugs.openjdk.org/browse/JDK-8334691) and [JDK-8337519](https://bugs.openjdk.org/browse/JDK-8337519).
>> 
>> Here is current output from debug VM on MacBook M1 (Aarch64):
>> 
>> External addresses table: 6 entries, 324 accesses
>> 0:      158 0x00000001082de0f0 : extn: vmClasses::_klasses+480
>> 1:       84 0x00000001082ddf20 : extn: vmClasses::_klasses+16
>> 2:       40 0x00000001082c4790 : extn: SharedRuntime::_partial_subtype_ctr
>> 3:       24 0x00000001082bdb04 : extn: JvmtiExport::_should_notify_object_alloc
>> 4:       18 0x0000000118384080 : stub: forward exception
>> 
>> 
>> on MacOS-x64:
>> 
>> External addresses table: 143 entries, 44405 accesses
>> 0:    11766 0x00000001047922a0 : extn: CompressedOops::_narrow_oop
>> 1:    11002 0x0000000104474384 : 'should not reach here'
>> 2:     9672 0x0000000104581a90 : extn: ClassLoader::file_name_for_class_name(char const*, int)::class_suffix+882068
>> 3:     2447 0x0000000104508005 : extn: ClassLoader::file_name_for_class_name(char const*, int)::class_suffix+383753
>> 4:     1916 0x000000010458188e : extn: ClassLoader::file_name_for_class_name(char const*, int)::class_suffix+881554
>> 
>> 
>> and on linux-x64:
>> 
>> External addresses table: 143 entries, 77297 accesses
>> 0:    22334 0x00007f35d5b9c000 : ''
>> 1:    19789 0x00007f35d55eea1f : 'should not reach here'
>> 2:    18366 0x00007f35d5747bb8 : 'MacroAssembler::decode_heap_oop: heap base corrupted?'
>> 3:     5036 0x00007f35d56e4d40 : 'uncommon trap returned which should never happen'
>> 4:     3643 0x00007f35d57479f8 : 'MacroAssembler::encode_heap_oop: heap base corrupted?'
>> 
>> 
>> Few points about difference in output:
>> 1. aarch64 does not use `ExternalAddess` or any relocation for messages (strings).
>> 2. `stub: forward exception` corresponds to `StubRoutines::forward_exception_entry()` for which C2 generates tail-call from [C2's stubs](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/generateOptoStub.cpp#L258C48-L258C87). It is difficult to convert it to `RuntimeAddress` because how relocation for constants in C2 are handled.
>> 3...
>
> Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add more missing cases and update Copyright year

And some addon change to keep RISC-V part up to date. Manually run workloads with -XX:+VerifyOops -XX:+TraceBytecodes.
@vnkozlov : Could you please help add it? Thanks.


diff --git a/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp b/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp
index e349eab3177..cd7a4ecf228 100644
--- a/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp
+++ b/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp
@@ -547,7 +547,7 @@ void MacroAssembler::_verify_oop(Register reg, const char* s, const char* file,
   }

   // call indirectly to solve generation ordering problem
-  ExternalAddress target(StubRoutines::verify_oop_subroutine_entry_address());
+  RuntimeAddress target(StubRoutines::verify_oop_subroutine_entry_address());
   relocate(target.rspec(), [&] {
     int32_t offset;
     la(t1, target.target(), offset);
@@ -592,7 +592,7 @@ void MacroAssembler::_verify_oop_addr(Address addr, const char* s, const char* f
   }

   // call indirectly to solve generation ordering problem
-  ExternalAddress target(StubRoutines::verify_oop_subroutine_entry_address());
+  RuntimeAddress target(StubRoutines::verify_oop_subroutine_entry_address());
   relocate(target.rspec(), [&] {
     int32_t offset;
     la(t1, target.target(), offset);
diff --git a/src/hotspot/cpu/riscv/templateInterpreterGenerator_riscv.cpp b/src/hotspot/cpu/riscv/templateInterpreterGenerator_riscv.cpp
index 769e4dc5ccc..f01945bc6a3 100644
--- a/src/hotspot/cpu/riscv/templateInterpreterGenerator_riscv.cpp
+++ b/src/hotspot/cpu/riscv/templateInterpreterGenerator_riscv.cpp
@@ -1111,8 +1111,8 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
   {
     Label L;
     __ ld(x28, Address(xmethod, Method::native_function_offset()));
-    address unsatisfied = (SharedRuntime::native_method_throw_unsatisfied_link_error_entry());
-    __ mv(t, unsatisfied);
+    ExternalAddress unsatisfied(SharedRuntime::native_method_throw_unsatisfied_link_error_entry());
+    __ la(t, unsatisfied);
     __ load_long_misaligned(t1, Address(t, 0), t0, 2); // 2 bytes aligned, but not 4 or 8

     __ bne(x28, t1, L);
@@ -1815,7 +1815,7 @@ void TemplateInterpreterGenerator::trace_bytecode(Template* t) {
   // the tosca in-state for the given template.

   assert(Interpreter::trace_code(t->tos_in()) != nullptr, "entry must have been generated");
-  __ call(Interpreter::trace_code(t->tos_in()));
+  __ rt_call(Interpreter::trace_code(t->tos_in()));
   __ reinit_heapbase();
 }

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

PR Comment: https://git.openjdk.org/jdk/pull/20412#issuecomment-2264437803