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

[Emanuel Peter]

On Fri, 23 May 2025 22:43:35 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:
> 
>   renaming

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

> 377:         _stress_rf_hook->add_req(loc);
> 378:       }
> 379:     }

Can you add a short code comment describing what you are doing here, please?

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

> 392:         _stress_rf_hook->add_req(stk);
> 393:       }
> 394:     }

A short code comment would be helpful

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

> 2220: 
> 2221:   if (StressReachabilityFences) {
> 2222:     // Keep all oop arguments alive until method return.

Why? Can you extend the comment a little?

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

> 42:  *   (0) initial set of RFs is materialized during parsing;
> 43:  *   (1) optimization pass during loop opts which eliminates redundant nodes and
> 44:  *     moves loop-invariant ones outside loops;

Suggestion:

 *   (1) optimization pass during loop opts which eliminates redundant nodes and
 *       moves loop-invariant ones outside loops;

I'd prever consistent indentation, but optional/question of taste

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

> 44:  *     moves loop-invariant ones outside loops;
> 45:  *   (2) reachability information is transferred to safepoint nodes (appended as edges after debug info);
> 46:  *   (3) reachability information from safepoints materialized as RF nodes attached to the safepoint node.

Can you expand the explanation a little, please? I don't really understand. Why do you do this? What does it achieve?

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

> 49:  *
> 50:  * It looks attractive to get rid of RF nodes early and transfer to safepoint-attached representation,
> 51:  * but it is not correct until loop opts are done.

Why is it not correct? What could go wrong? Why is it safe to do it after loop opts?

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

> 53:  * RF nodes may interfere with RA, so stand-alone RF nodes are eliminated and reachability information is
> 54:  * transferred to corresponding safepoints. When safepoints are pruned during macro expansion, corresponding
> 55:  * reachability info also goes away.

Is it ok that this info goes away?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2153895839
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2153896761
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2153906423
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2153913766
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2153924032
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2153925167
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2153927605