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

[Chad Rakoczy]

On Tue, 3 Jun 2025 12:06:31 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Chad Rakoczy has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix test copywrite
>
> src/hotspot/cpu/aarch64/relocInfo_aarch64.cpp line 89:
> 
>> 87:       x = trampoline;
>> 88:     }
>> 89:     call->set_destination(x);
> 
> I think I see what you're doing here, but it doesn't look right. At the very least it's a trap for maintainers, who don't expect the destination address to be discarded if the call doesn't reach.
> 
> When the call doesn't reach, I believe you're fixing up an internal call to point to its target in the new copy of the code. But this isn't right when calls are PC relative, is it? In that case it makes more sense to leave the call instruction alone rather than rewrite it.

This code specifically is for calls that were close enough to have an immediate offset before relocation but are now too far and require a trampoline. I agree this is probably not the best location for it and it should be moved up in the call stack (probably in `CallRelocation::fix_relocation_after_move`).

The code that fixes up the internal calls is [here](https://github.com/chadrako/jdk/blob/4e80e35959829ecf1579efc65b9525b2aff2be1f/src/hotspot/share/code/relocInfo.cpp#L380-L385). The issue is that the calls are PC relative but the logic in `CallRelocation::fix_relocation_after_move` changes the offset to point to the old nmethod’s code assuming it is an external routine. So it is incorrectly updating the offset when it needs to stay the same.

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

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