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

Tue Nov 11 11:58:20 UTC 2025

On Mon, 10 Nov 2025 02:27:15 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> This PR introduces C2 support for `Reference.reachabilityFence()`.
>> 
>> After [JDK-8199462](https://bugs.openjdk.org/browse/JDK-8199462) went in, it was discovered that C2 may break the invariant the fix relied upon [1]. So, this is an attempt to introduce proper support for `Reference.reachabilityFence()` in C2. C1 is left intact for now, because there are no signs yet it is affected.
>> 
>> `Reference.reachabilityFence()` can be used in performance critical code, so the primary goal for C2 is to reduce its runtime overhead as much as possible. The ultimate goal is to ensure liveness information is attached to interfering safepoints, but it takes multiple steps to properly propagate the information through compilation pipeline without negatively affecting generated code quality.
>> 
>> Also, I don't consider this fix as complete. It does fix the reported problem, but it doesn't provide any strong guarantees yet. In particular, since `ReachabilityFence` is CFG-only node, nothing explicitly forbids memory operations to float past `Reference.reachabilityFence()` and potentially reaching some other safepoints current analysis treats as non-interfering. Representing `ReachabilityFence` as memory barrier (e.g., `MemBarCPUOrder`) would solve the issue, but performance costs are prohibitively high. Alternatively, the optimization proposed in this PR can be improved to conservatively extend referent's live range beyond `ReachabilityFence` nodes associated with it. It would meet performance criteria, but I prefer to implement it as a followup fix.
>> 
>> Another known issue relates to reachability fences on constant oops. If such constant is GCed (most likely, due to a bug in Java code), similar reachability issues may arise. For now, RFs on constants are treated as no-ops, but there's a diagnostic flag `PreserveReachabilityFencesOnConstants` to keep the fences. I plan to address it separately. 
>> 
>> [1] https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/ref/Reference.java#L667
>> "HotSpot JVM retains the ref and does not GC it before a call to this method, because the JIT-compilers do not have GC-only safepoints."
>> 
>> Testing:
>> - [x] hs-tier1 - hs-tier8
>> - [x] hs-tier1 - hs-tier6 w/ -XX:+StressReachabilityFences -XX:+VerifyLoopOptimizations
>> - [x] java/lang/foreign microbenchmarks
>
> Vladimir Ivanov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Revise RF redunancy & auto-boxed primitives handling
>   Cleanups

Nice improvements. I only looked over the recent changes. I'll try to have a look at the whole change later on.

src/hotspot/share/opto/parse1.cpp line 1250:

> 1248:       Node* loc = local(idx);
> 1249:       if (loc->bottom_type()->isa_oopptr() != nullptr &&
> 1250:           !is_auto_boxed_primitive(loc)) { // ignore auto-boxed primitives

I wonder if randomizing this would shake out more interesting patterns?

src/hotspot/share/opto/reachability.cpp line 150:

> 148:     return false; // not a real safepoint
> 149:   } else if (sfpt->is_CallStaticJava() && sfpt->as_CallStaticJava()->is_uncommon_trap()) {
> 150:     return false; // uncommon traps are exit points

Can we even hit this situation with a traversal from below? Just curious ;)

src/hotspot/share/opto/reachability.cpp line 208:

> 206: //---------------------------- Phase 1 ---------------------------------
> 207: // Optimization pass over reachability fences during loop opts.
> 208: // Eliminate redundant RFs and move RFs with loop-invariant referent out of the loop.

You removed the `find_redundant_rfs` case. Is the comment still accurate?

src/hotspot/share/opto/reachability.cpp line 219:

> 217:   for (int i = 0; i < C->reachability_fences_count(); i++) {
> 218:     ReachabilityFenceNode* rf = C->reachability_fence(i);
> 219:     assert(!rf->is_redundant(igvn()), "required");

Why can we assume this? Is this guaranteed by IGVN?

src/hotspot/share/opto/reachability.cpp line 220:

> 218:     ReachabilityFenceNode* rf = C->reachability_fence(i);
> 219:     assert(!rf->is_redundant(igvn()), "required");
> 220:     // Move RFs out of counted loops when possible.

Is this limited to counted loops? Ah `unique_loop_exit_or_null` restricts it.
That seems fine, I'm just worried that we may at some point allow non-counted loops, and then the comment will be incorrect.

src/hotspot/share/opto/reachability.cpp line 228:

> 226:       for (IdealLoopTree* outer_loop = lpt->_parent;
> 227:            outer_loop->is_invariant(referent) && outer_loop->unique_loop_exit_or_null() != nullptr;
> 228:            outer_loop = outer_loop->_parent) {

Out of curiosity: is it always desirable to move out as far as possible? Or are there downsides?

src/hotspot/share/opto/reachability.cpp line 335:

> 333: // Phase 2: migrate reachability info to safepoints.
> 334: // All RFs are replaced with edges from corresponding referents to interfering safepoints.
> 335: // Interfering safepoints are safepoint nodes which are reachable from the RF to its referent through CFG.

Seems you don't do it for ALL any more, you drop those that `dominates_another_rf`. You should probably adapt the comment here.

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

PR Review: https://git.openjdk.org/jdk/pull/25315#pullrequestreview-3447403459
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2513717114
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2513740164
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2513873982
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2513809786
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2513822750
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2513854603
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2513952780