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

[Vladimir Ivanov]

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

> could we just go through _reachability_fences, and hack the graph and clean up with IGVN? Or do we really need the loop state to do this successfully?

RF elimination needs control for referent to enumerate all interfering safepoints. 

Theoretically, it's possible to use a conservative estimate, but then:
 (1) it can worsen the result (by enumerating more interfering safepoints than needed); and
 (2) build an unschedulable graph if referent doesn't dominate safepoint node (if estimate is way too conservative). 

IMO it's safer to build full dominator tree here.  

> It probably has a performance impact, right? Have you measured that? 

It does have a noticeable cost. On my laptop it bumps the time spent doing RF processing from 170ms to 210ms

$ java -Xcomp -XX:-TieredCompilation -XX:+CITime -XX:+UnlockDiagnosticVMOptions -XX:-StressReachabilityFences

         IdealLoop:             0.173 s
           ReachabilityFence:   0.000 s
             Optimize:          0.000 s
             Eliminate:         0.000 s
``` 
vs

$ java -Xcomp -XX:-TieredCompilation -XX:+CITime -XX:+UnlockDiagnosticVMOptions -XX:+StressReachabilityFences

         IdealLoop:             0.212 s
           ReachabilityFence:   0.030 s
             Optimize:          0.004 s
             Eliminate:         0.004 s
``` 

I reimplemented it to piggyback on the last loop optimization attempt if there's any and it drastically improves the situation:

$ java -Xcomp -XX:-TieredCompilation -XX:+CITime -XX:+UnlockDiagnosticVMOptions -XX:+StressReachabilityFences

         IdealLoop:             0.193 s
           ReachabilityFence:   0.009 s
             Optimize:          0.003 s
             Eliminate:         0.004 s

> src/hotspot/share/opto/loopTransform.cpp line 66:
> 
>> 64: //------------------------------unique_loop_exit_or_null----------------------
>> 65: // Return the loop-exit projection if it is unique.
>> 66: Node* IdealLoopTree::unique_loop_exit_or_null() {
> 
> I suggested it here:
> https://github.com/openjdk/jdk/pull/25315#discussion_r2149677594
> Can we change the return type to `IfProjNode`?
> 
> Also: when is it possible that there are none or multiple loop exits?
> Can you add a comment below where you return nullptr?

Done.

> src/hotspot/share/opto/parse1.cpp line 2233:
> 
>> 2231:       insert_reachability_fence(referent);
>> 2232:     }
>> 2233:   }
> 
> Comments look better, thanks :)
> 
> But `StressReachabilityFences` seems to promise that it should happen randomly. Did you want to do that or adjust the flag comment?

I adjusted flag comment.

> src/hotspot/share/opto/reachability.cpp line 49:
> 
>> 47:  *
>> 48:  * It is tempting to directly attach referents to interfering safepoints right from the beginning, but it
>> 49:  * doesn't play well with some optimizations C2 does.
> 
> Do you have an example for such optimizations?

Loop-invariant code motion is one example. Do you want me to add it to the comment?

After parsing is over, the IR is in valid state, but loop optimizations are the primary reason why it can be broken later.

> src/hotspot/share/opto/reachability.cpp line 67:
> 
>> 65:  * RF nodes may interfere with RA, so stand-alone RF nodes are eliminated and their referents are
>> 66:  * transferred to corresponding safepoints (phase #2). When safepoints are pruned during macro expansion,
>> 67:  * corresponding reachability edges also go away.
> 
> Spell our RA on first use. Make more clear that this is why we eliminate RF before RA.
> Suggestion:
> 
>  * RF nodes may interfere with register allocation (RA), hence we eliminate RF nodes and transfer their
>  * referents  to corresponding safepoints (phase #2). When safepoints are pruned during macro expansion,
>  * corresponding reachability edges also go away.
> 
> `reachability edges also go away` ... and that is ok why? Sketch of what you could write, is it correct?
> - reachability only needs to be correct at SafePoints. If all the SafePoints are removed for a referent, then we don't need to ensure its reachablility.

Applied your suggested change and elaborated the comment.

> the very same similar way sounds a little funny. I
Fixed. 

>  What is the issue with the edges being attached to safepoints here?

The issue is safepoint-attached representation conflicts with derived oops representation. There's no way to distinguish between them. As of now, VM treats post-debug info edges as representing derived oops which is completely wrong when there are reachability edges present. More work is needed to support both cases.

> src/hotspot/share/opto/reachability.cpp line 438:
> 
>> 436:   if (!OptimizeReachabilityFences) {
>> 437:     return false;
>> 438:   }
> 
> Can this ever fail? Could it be an assert?

Done.

> src/hotspot/share/opto/reachability.cpp line 441:
> 
>> 439: 
>> 440:   Unique_Node_List redundant_rfs;
>> 441:   Node_List worklist;
> 
> Not sure if necessary, but maybe good practice anyway: add `ResourceMark`.

Done.

> src/hotspot/share/opto/reachability.cpp line 453:
> 
>> 451:         SafePointNode* sfpt = safepoints.pop()->as_SafePoint();
>> 452:         assert(is_dominator(get_ctrl(referent), sfpt), "");
>> 453:         assert(sfpt->req() == rf_start_offset(sfpt), "");
> 
> Is this the only reason we need this to happend during LoopOpts - i.e. that we can call `get_ctrl` and `is_dominator`?
> 
> Because it is potentially a lot of overhead to create the whole loop-opts structures just for this.

It's solely for `get_ctrl(referent)` call in `enumerate_interfering_sfpts()`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2337971541
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2337972022
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2337978893
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2334848196
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2334876169
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2337985581
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2337997889
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2337998906
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2337994039