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

[Emanuel Peter]

On Tue, 9 Sep 2025 21:45:00 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> Also: you promise that it happens randomly. But it seems to be added deterministically everywhere. Did I miss something?
>
> Sorry for the confusion. Reworded the comment. I didn't intend to make it truly random. The idea was to automatically insert RF nodes during parsing to stress the implementation. It doesn't slow down compilation times that much, so aggressive insertion just works.

@iwanowww Ok, makes sense. I wonder though if we should consider random insertion.
It would also be nice to document somewhere that this only really tests the internal mechanism of ReachabilityFence. It does not really stess the case where we should have had a ReachabilityFence but fail to have one. Instead, we just insert more than we need. So we don't really expect this to trigger bugs with missing RFs.

>> 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.

Maybe it could be worth putting some comments around `guaranteed_safepoint` that describe the logic of it, while you have the advantage of understanding what it means?

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

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