RFR: 8290892: C2: Intrinsify Reference.reachabilityFence [v3]

[Vladimir Ivanov]

On Mon, 8 Sep 2025 12:55:36 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Well, it's a SafePointNode class after all. I lifted it from `CallNode` subclass to avoid elaborate check on SafePoint nodes (!is_Call() || as_Call() && guaranteed_safepoint()`)).
>> 
>> If some node extends SafePointNode, but doesn't keep JVM state, it has to communicate it to users one way or another. And changing the default doesn't improve the situation IMO: reporting a safepoint node as a non-safepoint is still a bug.
>
> Hmm. The way it is formulated it sounds more like:
> - `true` -> we are guaranteed that it is a safepoint.
> - `false` -> it may or may not be a safepoint - no guarantees.
> Am I understanding this right?
> 
> If yes, then it would make more sense to have a default that is `no guarantee`. But maybe that makes things more complicated in other ways. All I'm saying it makes me nervous ;)

You are right. I studied the code and `guaranteed_safepoint()` behaves as you described. It doesn't work for RF purposes, so I migrated the code to `sfpt->jvms() != nullptr` check and fixed a bug along the way. The changes related to `guaranteed_safepoint()` are reverted.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2341997278