RFR: 8316694: Implement relocation of nmethod within CodeCache [v32]

[Evgeny Astigeevich]

On Thu, 26 Jun 2025 16:20:44 GMT, Chad Rakoczy <duke at openjdk.org> wrote:

>> src/hotspot/cpu/aarch64/nativeInst_aarch64.cpp line 90:
>> 
>>> 88:   // Patch the constant in the call's trampoline stub.
>>> 89:   address trampoline_stub_addr = get_trampoline();
>>> 90:   if (trampoline_stub_addr != nullptr && dest != trampoline_stub_addr) {
>> 
>> I think you will not need the checks if you rewrite the code as follows:
>> ```c++
>> address addr_call = ...;
>> assert();
>> 
>> if (!Assembler::reachable_from_branch_at(addr_call, dest)) {
>>   address trampoline_stub_addr = get_trampoline();
>>   assert (trampoline_stub_addr != nullptr, "we need a trampoline");
>>   assert (! is_NativeCallTrampolineStub_at(dest), "chained trampolines");
>>   nativeCallTrampolineStub_at(trampoline_stub_addr)->set_destination(dest);
>>   dest = trampoline_stub_addr;
>> }
>> set_destination(dest);
>> ICache::invalidate_range(addr_call, instruction_size);
>> 
>> 
>> If `dest` is a trampoline in the current nmethod, it is always reachable. So you will not go into setting trampoline's target to itself. Also we will call `get_trampoline`, which involves `CodeCache::find_blob` and ` a traversal of relocations, only if we need a trampoline.
>
> I would need to check the assumptions that other callers make about this function. In the current state it updates the trampoline regardless if the branch is reachable or not. With your change it would require the caller to also update the trampoline to make sure it is not stale.

@theRealAph When we don't need a trampoline (a call site is a direct call), we update the trampoline to have the same destination as the call site. I have not found places in Hotspot relying on this. 
Do you remember why we are doing this? Is it Ok not to update trampolines in the case of reachable destinations?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23573#discussion_r2176748370