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

[Vladimir Ivanov]

On Mon, 16 Jun 2025 09:28:59 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/callnode.cpp line 950:
>> 
>>> 948:               case CatchProjNode::catch_all_index:    projs->catchall_catchproj    = cpn; break;
>>> 949:               default: {
>>> 950:                 assert(cpn->_con > 1, ""); // exception table; rethrow case
>> 
>> Can we please turn this into a helpful assert message?
>
> Can you quickly comment why you changed this?

Some call nodes inspected during `expand_reachability_fences` demonstrate this IR shape where some exception table projections are directly attached to the call node.

Looks like a missed case in `CallNode::extract_projections` we simply never hit before.

>> src/hotspot/share/opto/loopnode.hpp line 1485:
>> 
>>> 1483:   void remove_rf(Node* rf);
>>> 1484: #ifdef ASSERT
>>> 1485:   bool has_redundant_rfs(Unique_Node_List& ignored_rfs, bool rf_only);
>> 
>> I would prefer if all the method names spelled out `reachability_fences` instead of `rf / rfs`.
>
> The arguments are less important for me.

There are 2 types of methods here: internal ones (used solely in `reachability.cpp`) and those which are called from loop optimization code (`optimize_reachability_fences` and `eliminate_reachability_fences`). 

IMO it's counter-productive to repeatedly spell out what "RF" means inside `reachability.cpp`, so I kept the names intact. I split the declarations into public and private ones to stress the distinction.

>> src/hotspot/share/opto/reachability.cpp line 46:
>> 
>>> 44:  *     moves loop-invariant ones outside loops;
>>> 45:  *   (2) reachability information is transferred to safepoint nodes (appended as edges after debug info);
>>> 46:  *   (3) reachability information from safepoints materialized as RF nodes attached to the safepoint node.
>> 
>> Can you expand the explanation a little, please? I don't really understand. Why do you do this? What does it achieve?
>
> It could be helpful if you wrote a paragraph (maybe at the top), about the interaction of SafePoint and ReachabilityFence. And you should also define "reachability information", I don't yet understand what that entails.

I elaborated the description a bit and added more details. Let me know how it reads now.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2320108310
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2320132768
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2320139929