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

[Evgeny Astigeevich]

On Wed, 28 May 2025 20:43:03 GMT, Chad Rakoczy <duke at openjdk.org> wrote:

>>> The patched call instruction will be incorrect.
>> 
>> That's not entirely correct. The null trampoline check is needed because on debug builds branches of distance >2M will fall into the `if (!Assembler::reachable_from_branch_at(addr(), x))` block but Graal would not have generated a trampoline for that call because it is still <128M. It is still safe to use that distance but it is just different than what HotSpot expects
>
> If we want to guarantee that a trampoline exists if `Assembler::reachable_from_branch_at` fails we would need to update Graal to use the check as well

> The null trampoline check is needed because on debug builds branches of distance >2M will fall into the if (!Assembler::reachable_from_branch_at(addr(), x)) block but Graal would not have generated a trampoline for that call because it is still <128M. It is still safe to use that distance but it is just different than what HotSpot expects

This logic looks strange to me.
You are saying that a trampoline is only null in case of Graal but dest is always valid in this case.
This is a bug in Graal: it always uses 128M branch range despite Hotspot can change the range to smaller values in debug builds.
When Graal fixes the bug you will have undefined behaviour in this place.
We must handle the situation where no trampoline is available.
Options:
1. This is a bug in code generation. If the bug can be easy to reproduce with debug builds, use assert. If no, use guarantee.
2. This is an expected case. We need to generate a trampoline. This can be complicated.

I think it's a bug situation.

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

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