RFR: 8337396: Cleanup usage of ExternalAddess [v2]
Andrew Dinn
adinn at openjdk.org
Thu Aug 1 15:23:32 UTC 2024
On Thu, 1 Aug 2024 03:19:01 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 missed ExternalAddress changes
I think I found a few more places in aarch64 and x86 where we need to use RuntimeAddress. There may be similar problems in the other arch implementations:
diff --git a/src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp b/src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp
index 90c7ca6f08a..c6b078c3c7d 100644
--- a/src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp
+++ b/src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp
@@ -179,7 +179,7 @@ void InterpreterRuntime::SignatureHandlerGenerator::generate(uint64_t fingerprin
iterate(fingerprint);
// return result handler
- __ lea(r0, ExternalAddress(Interpreter::result_handler(method()->result_type())));
+ __ lea(r0, RuntimeAddress(Interpreter::result_handler(method()->result_type())));
__ ret(lr);
__ flush();
diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
index f90aefc8fd3..73da947c318 100644
--- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
+++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
@@ -1879,7 +1879,7 @@ void MacroAssembler::_verify_oop(Register reg, const char* s, const char* file,
movptr(rscratch1, (uintptr_t)(address)b);
// call indirectly to solve generation ordering problem
- lea(rscratch2, ExternalAddress(StubRoutines::verify_oop_subroutine_entry_address()));
+ lea(rscratch2, RuntimeAddress(StubRoutines::verify_oop_subroutine_entry_address()));
ldr(rscratch2, Address(rscratch2));
blr(rscratch2);
@@ -1918,7 +1918,7 @@ void MacroAssembler::_verify_oop_addr(Address addr, const char* s, const char* f
movptr(rscratch1, (uintptr_t)(address)b);
// call indirectly to solve generation ordering problem
- lea(rscratch2, ExternalAddress(StubRoutines::verify_oop_subroutine_entry_address()));
+ lea(rscratch2, RuntimeAddress(StubRoutines::verify_oop_subroutine_entry_address()));
ldr(rscratch2, Address(rscratch2));
blr(rscratch2);
diff --git a/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp b/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp
index 89f5fbd281b..215f1b6453b 100644
--- a/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp
+++ b/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp
@@ -1337,7 +1337,7 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
{
Label L;
__ ldr(r10, Address(rmethod, Method::native_function_offset()));
- address unsatisfied = (SharedRuntime::native_method_throw_unsatisfied_link_error_entry());
+ RuntimeAddress unsatisfied(SharedRuntime::native_method_throw_unsatisfied_link_error_entry());
__ mov(rscratch2, unsatisfied);
__ ldr(rscratch2, rscratch2);
__ cmp(r10, rscratch2);
@@ -1432,7 +1432,7 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
// hand.
//
__ mov(c_rarg0, rthread);
- __ mov(rscratch2, CAST_FROM_FN_PTR(address, JavaThread::check_special_condition_for_native_trans));
+ __ mov(rscratch2, RuntimeAddress(CAST_FROM_FN_PTR(address, JavaThread::check_special_condition_for_native_trans)));
__ blr(rscratch2);
__ get_method(rmethod);
__ reinit_heapbase();
@@ -1461,7 +1461,7 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
{
Label no_oop;
- __ adr(t, ExternalAddress(AbstractInterpreter::result_handler(T_OBJECT)));
+ __ adr(t, RuntimeAddress(AbstractInterpreter::result_handler(T_OBJECT)));
__ cmp(t, result_handler);
__ br(Assembler::NE, no_oop);
// Unbox oop result, e.g. JNIHandles::resolve result.
@@ -1482,7 +1482,7 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
__ push_call_clobbered_registers();
__ mov(c_rarg0, rthread);
- __ mov(rscratch2, CAST_FROM_FN_PTR(address, SharedRuntime::reguard_yellow_pages));
+ __ mov(rscratch2, RuntimeAddress(CAST_FROM_FN_PTR(address, SharedRuntime::reguard_yellow_pages)));
__ blr(rscratch2);
__ pop_call_clobbered_registers();
@@ -2085,7 +2085,7 @@ void TemplateInterpreterGenerator::trace_bytecode(Template* t) {
assert(Interpreter::trace_code(t->tos_in()) != nullptr,
"entry must have been generated");
- __ bl(Interpreter::trace_code(t->tos_in()));
+ __ bl(RuntimeAddress(Interpreter::trace_code(t->tos_in())));
__ reinit_heapbase();
}
diff --git a/src/hotspot/cpu/x86/interpreterRT_x86_32.cpp b/src/hotspot/cpu/x86/interpreterRT_x86_32.cpp
index a9b96c22427..f9adc8b49a9 100644
--- a/src/hotspot/cpu/x86/interpreterRT_x86_32.cpp
+++ b/src/hotspot/cpu/x86/interpreterRT_x86_32.cpp
@@ -93,7 +93,7 @@ void InterpreterRuntime::SignatureHandlerGenerator::generate( uint64_t fingerpri
iterate(fingerprint);
// return result handler
__ lea(rax,
- ExternalAddress((address)Interpreter::result_handler(method()->result_type())));
+ RuntimeAddress((address)Interpreter::result_handler(method()->result_type())));
// return
__ ret(0);
__ flush();
diff --git a/src/hotspot/cpu/x86/interpreterRT_x86_64.cpp b/src/hotspot/cpu/x86/interpreterRT_x86_64.cpp
index 7e390564f4c..ec78445dab6 100644
--- a/src/hotspot/cpu/x86/interpreterRT_x86_64.cpp
+++ b/src/hotspot/cpu/x86/interpreterRT_x86_64.cpp
@@ -295,7 +295,7 @@ void InterpreterRuntime::SignatureHandlerGenerator::generate(uint64_t fingerprin
iterate(fingerprint);
// return result handler
- __ lea(rax, ExternalAddress(Interpreter::result_handler(method()->result_type())));
+ __ lea(rax, RuntimeAddress(Interpreter::result_handler(method()->result_type())));
__ ret(0);
__ flush();
diff --git a/src/hotspot/cpu/x86/templateInterpreterGenerator_x86.cpp b/src/hotspot/cpu/x86/templateInterpreterGenerator_x86.cpp
index fe2bf67afc9..594216860f3 100644
--- a/src/hotspot/cpu/x86/templateInterpreterGenerator_x86.cpp
+++ b/src/hotspot/cpu/x86/templateInterpreterGenerator_x86.cpp
@@ -1002,7 +1002,7 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
{
Label L;
__ movptr(rax, Address(method, Method::native_function_offset()));
- ExternalAddress unsatisfied(SharedRuntime::native_method_throw_unsatisfied_link_error_entry());
+ RuntimeAddress unsatisfied(SharedRuntime::native_method_throw_unsatisfied_link_error_entry());
__ cmpptr(rax, unsatisfied.addr(), rscratch1);
__ jcc(Assembler::notEqual, L);
__ call_VM(noreg,
@@ -1075,8 +1075,8 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
{ Label L;
Label push_double;
- ExternalAddress float_handler(AbstractInterpreter::result_handler(T_FLOAT));
- ExternalAddress double_handler(AbstractInterpreter::result_handler(T_DOUBLE));
+ RuntimeAddress float_handler(AbstractInterpreter::result_handler(T_FLOAT));
+ RuntimeAddress double_handler(AbstractInterpreter::result_handler(T_DOUBLE));
__ cmpptr(Address(rbp, (frame::interpreter_frame_oop_temp_offset + 1)*wordSize),
float_handler.addr(), noreg);
__ jcc(Assembler::equal, push_double);
@@ -1167,7 +1167,7 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
{
Label no_oop;
- __ lea(t, ExternalAddress(AbstractInterpreter::result_handler(T_OBJECT)));
+ __ lea(t, RuntimeAddress(AbstractInterpreter::result_handler(T_OBJECT)));
__ cmpptr(t, Address(rbp, frame::interpreter_frame_result_handler_offset*wordSize));
__ jcc(Assembler::notEqual, no_oop);
// retrieve result
-------------
PR Comment: https://git.openjdk.org/jdk/pull/20412#issuecomment-2263340905
More information about the hotspot-dev
mailing list