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

[Vladimir Ivanov]

On Tue, 11 Nov 2025 10:40:22 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Vladimir Ivanov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Revise RF redunancy & auto-boxed primitives handling
>>   Cleanups
>
> 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?

It could be the case when domination-based redundancy analysis was in place. In the current version, I don't see any benefit in test coverage.

> 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 ;)

With the upwards traversal of CFG, uncommon traps shouldn't be enumerated. But it's clearer to still keep the check in `is_interfering_sfpt_candidate` since it's agnostic of the details about traversal. I added an assert in the caller.

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

Ok, adjusted.

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

Yes, the assumption is that last IGVN pass removed all redundant RFs and the graph hasn't changed for new cases to occur.

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

The code assumes a single exit point, so if non-counted loops are supported, then the code have to be adjusted anyway.

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

The traversal enumerates all encountered safepoints which pass `is_interfering_sfpt_candidate`. It's the caller which avoids the traversals for redundant nodes.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2520998205
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2521005809
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2521013812
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2521012642
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2521013291
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2521014079