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

[Vladimir Ivanov]

On Fri, 7 Nov 2025 09:59:29 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Vladimir Ivanov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   update
>
> src/hotspot/share/opto/loopnode.cpp line 5147:
> 
>> 5145:     assert(do_expensive_nodes || do_optimize_reachability_fences, "why are we here?");
>> 5146:     // Use a change to optimize reachability fence nodes irrespective of
>> 5147:     // whether loop optimizations are performed or not.
> 
> What do you mean by `Use a change`?

Fixed.

> src/hotspot/share/opto/reachability.cpp line 186:
> 
>> 184:     return false; // uncommon traps are exit points
>> 185:   }
>> 186:   return true;
> 
> Suggestion:
> 
>   // By default, we return a conservative answer, and assume it could interfere.
>   return true;

I updated the comment.

I wouldn't say it produces a conservative answer, since the query is for "interfering safepoint candidate" and RF-agnostic. It becomes accurate when applied during CFG traversal.

> src/hotspot/share/opto/reachability.cpp line 446:
> 
>> 444:   ResourceMark rm;
>> 445:   Unique_Node_List redundant_rfs;
>> 446:   Node_List worklist;
> 
> Looks like this `worklist` is really a list of `<sfpt,referent>` pairs.
> 
> Consider making it a `GrowableArray` with a pair type and also consider giving it a more descriptive name, calling it by whatever we collect in it. Maybe `sfpt_referent_pairs`?
> 
> Also: why do we need this worklist, and not just attach the referent to the sfpt eagerly? And: can it be that we add the same pair multiple times? Is that intentional?

Sounds good, I applied your suggestion.

> Also: why do we need this worklist, and not just attach the referent to the sfpt eagerly?

There's an assert to ensure there are no non-debug edges on discovered safepoints. It'd be harder to ensure the invariant if safepoints are modified during the analysis.  

> And: can it be that we add the same pair multiple times? Is that intentional?

Good catch. I added the missing check guarding `add_req` call.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2505816538
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2505815407
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2505815783