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

Fri Sep 12 14:12:42 UTC 2025

On Thu, 11 Sep 2025 18:18:13 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:
> 
>   Minor fix

@iwanowww Thanks for all the updates, and the presentation on Tuesday in our staff-meeting!

src/hotspot/share/opto/callGenerator.cpp line 620:

> 618:     // Inlining logic doesn't expect any extra edges past debug info and fails with
> 619:     // an assert in SafePointNode::grow_stack.
> 620:     assert(endoff == call->req(), "reachability edges not supported");

Could we trip over this assert by modifying the reproducer, and add some method somewhere that gets inlined late?

src/hotspot/share/opto/compile.hpp line 110:

> 108:   LoopOptsNone,
> 109:   LoopOptsMaxUnroll,
> 110:   LoopOptsEliminateRFs,

With the additional flags,  I think we now need some kind of documentation here. I'm losing a bit the overview - and maybe never really had it.

src/hotspot/share/opto/loopnode.cpp line 5341:

> 5339:     C->print_method(PHASE_ELIMINATE_REACHABILITY_FENCES, 2);
> 5340:     assert(C->reachability_fences_count() == 0, "no RF nodes allowed");
> 5341:   }

Can we somehow assert that we now really will never do loop-opts again?
Why are you checking for `_mode == LoopOptsDefaultFinal` and not for `LoopOptsEliminateRFs`?
If that was a bug, then more verification would be extra justified ;)

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

> 50:  *
> 51:  * Instead, reachability representation transitions through multiple phases:
> 52:  *   (0) initial set of RFs is materialized during parsing;

Suggestion:

 *   (0) initial set of RFs is materialized during parsing, by intrinsifying calls to Reference.reachabilityFence;

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

> 52:  *   (0) initial set of RFs is materialized during parsing;
> 53:  *   (1) optimization pass during loop opts eliminates redundant RF nodes and
> 54:  *       moves the ones with loop-invariant referents outside loops;

Suggestion:

 *   (1) optimization pass during loop opts eliminates redundant RF nodes and
 *       moves the ones with loop-invariant referents outside (after) loops;

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

> 65:  * Live ranges of values are routinely extended during loop opts. And it can break the invariant that
> 66:  * all interfering safepoints contain the referent in their oop map. (If an interfering safepoint doesn't
> 67:  * keep the referent alive, then it becomes possible for the referent to be prematurely GCed.)

Can we have a concrete example. I thought of a store that is sunk out of the loop. But of course that should not cross a SafePoint on the way either. So then that's not a good argument. Do you have one that works?

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

> 68:  *
> 69:  * After loop opts are over, it becomes possible to reliably enumerate all interfering safe points and
> 70:  * ensure the referent present in their oop maps.

Suggestion:

 * After loop opts are over, it becomes possible to reliably enumerate all interfering safe points and
 * to ensure that the referent is present in their oop maps.

Grammar. Maybe you need to fix it in a different way if it does not match the intended semantics ;)

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

> 79:  * (c) Unfortunately, it's not straightforward to stay with safepoint-attached representation till the very end,
> 80:  * because information about derived oops is attached to safepoints in a similar way. So, for now RFs are
> 81:  * rematerialized at safepoints before RA (phase #3).

I still don't understand this. What is similar to what? And why is that a problem?

src/hotspot/share/opto/reachability.hpp line 32:

> 30: #include "opto/type.hpp"
> 31: 
> 32: //------------------------ReachabilityFenceNode--------------------------

Suggestion:

// Represents a Reference.reachabilityFence call
// See documentation in reachability.cpp

test/hotspot/jtreg/compiler/c2/TestReachabilityFence.java line 40:

> 38:  * @run main/othervm -Xbatch compiler.c2.TestReachabilityFence
> 39:  */
> 40: public class TestReachabilityFence {

This test seems very important to me. Can you please add some extra code comments, about what goes wrong before the fix, i.e. if RF are not present?

Maybe some explanation about what it took to write this test, so that we can build on that to extend the test later?

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

PR Review: https://git.openjdk.org/jdk/pull/25315#pullrequestreview-3216761112
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2344292203
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2344381871
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2344313204
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2344334081
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2344337061
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2344345310
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2344349802
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2344355280
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2344359681
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2344374341