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

[Dean Long]

On Tue, 17 Jun 2025 21:05:12 GMT, Dean Long <dlong at openjdk.org> wrote:

>>> Isn't this logic only required because of Graal (JDK-8358096)?
>> 
>> I think it is needed for HotSpot as well. Just because a trampoline was generated for a call does mean it is being used. We still need this check in the event that there is a direct call that no longer reaches.
>> 
>>> For nmethod to nmethod relocation, I think you want a customized version of this function instead of trying to use it as-is.
>> 
>> I moved this logic to [CallRelocation::fix_relocation_after_move()](https://github.com/chadrako/jdk/blob/03bfce8779a0f9c9a2c276fc6cf084698e6725d7/src/hotspot/share/code/relocInfo.cpp#L373-L402). It first checks if the destination is within the source and also check if the call is too far.
>> 
>>> otherwise recursive self calls to the Java method would not get relocated correctly
>> 
>> What do you mean by this? I don't see how recursive calls would behave differently. There is a check for intra-nmethod calls which I think would cover this case
>
>> What do you mean by this? I don't see how recursive calls would behave differently. There is a check for intra-nmethod calls which I think would cover this case
> 
> OK, I didn't see that check when I took a quick look.  If it already works as intended, great.

> We still need this check in the event that there is a direct call that no longer reaches.

OK, I didn't realize that was what Relocation::pd_set_call_destination() was doing.  I think it would be better for the CPU-specific code to take care of that, rather than the shared code.  We already have functions like NativeCall::set_destination_mt_safe() that do the right thing regarding trampolines.  I think this could be refactored into a commonm function that Relocation::pd_set_call_destination() could also use.  Sorry for the churn, but hopefully we are converging on a solution.  I thought I had done the refactoring for 8321509, but it looks like I went with the simply fix at the time of adding a parameter to set_destination_mt_safe() to make it lock-free.

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

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