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

[Christian Hagedorn]

On Thu, 22 May 2025 22:19:25 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> src/hotspot/share/opto/phasetype.hpp line 88:
>> 
>>> 86:   flags(PHASEIDEALLOOP2,                "PhaseIdealLoop 2") \
>>> 87:   flags(PHASEIDEALLOOP3,                "PhaseIdealLoop 3") \
>>> 88:   flags(OPTIMIZE_RF,                    "Optimize Reachability Fences") \
>> 
>> Another drive-by comment: I suggest to use the full word since most people are probably not aware of this abbreviation when looking at graph dumps in IGV. You should also add this phase to the IR framework [CompilePhases](https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/compiler/lib/ir_framework/CompilePhase.java).
>> 
>> Suggestion:
>> 
>>   flags(OPTIMIZE_REACHABILITY_FENCES,   "Optimize Reachability Fences") \
>
> IMO it doesn't clarify things much. It's enum constant name and it's not shown in IGV.
> 
> It is used in a single place [1] where it's clear what it refers to. 
> 
> [1]
> 
>   { // No more loop opts. It is safe to eliminate reachability fence nodes.
>     TracePhase tp(_t_idealLoop);
>     PhaseIdealLoop::optimize(igvn, LoopOptsEliminateRFs);
>     print_method(PHASE_OPTIMIZE_RF, 2);
>     if (failing())  return;
>   }

That's a good point, there it really does not matter. Another thought I've just had: When you add it to the `CompilePhase` class in the IR framework and it would be nice to have the same name. There, it would be beneficial to have the full name since people then only see `CompilePhase::OPTIMIZE_RF`.

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

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