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

[Vladimir Ivanov]

On Tue, 4 Nov 2025 08:36:21 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> `is_significant_sfpt()` encodes a white list consisting of cases which can be safely ignored when it comes to reachability tracking. An overlooked case is a missed optimization opportunity. 
>> 
>>> But can you be more specific?
>> 
>> Are you suggesting to expand the comment or change the name?
>> 
>> Speaking of the name, it's a local definition inside `reachabiltiy.cpp`:
>> 
>> // Detect safepoint nodes which are important for reachability tracking purposes.
>> static bool is_significant_sfpt(Node* n) {
>> 
>> 
>> `Significant` term is declarative. Alternatively, an imperative term (like, "ignore" and `ignore_sfpt`) can be used. But I find it the current version clearer.
>
>> is_significant_sfpt() encodes a white list consisting of cases which can be safely ignored when it comes to reachability tracking. An overlooked case is a missed optimization opportunity.
> 
> Sounds good. Can you add a code comment for that, please?
> 
> Ok, I'm fine with keeping the name. But it might make sense to link to where the "significance" term is defined. Because otherwise it is a concept without any clear definition, and hard for the reader to understand. You have to infer the definition from the usage, and that often leads to unclear definitions that shift over time, and eventually the concept even is incoherent. A clear definition can also help if we have a bug: we can clearly see what we missed and what we might have to do to fix it.

Ok, I got rid of "significant" in favor of "interfering" (new name is `is_interfering_sfpt_candidate`).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2501928414