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

[Emanuel Peter]

On Mon, 8 Sep 2025 12:29:15 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Vladimir Ivanov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   whitespaces
>
> src/hotspot/share/opto/callGenerator.cpp line 623:
> 
>> 621:       return; // keep the original call node as the holder of reachability info
>> 622:     }
>> 623:   }
> 
> Maybe that's just me. But people use the assert messages both in positive and negative ways, and so this is a bit ambiguous. Maybe you can write:
> `no reachability edge should be present`
> 
> I'm still a bit unsure what the `SafePointNode::grow_stack` comment means.
> In the previous comment https://github.com/openjdk/jdk/pull/25315#discussion_r2320120466 you explained more. Why not add that here instead?

I'm also not sure yet why there is a difference between incremental inlining and regular inlining.
Do you think it would make sense to explain that here, or is it explained elsewhere?

> src/hotspot/share/opto/macro.cpp line 973:
> 
>> 971:         _igvn._worklist.push(ac);
>> 972:       } else if (use->is_ReachabilityFence() && OptimizeReachabilityFences) {
>> 973:         use->as_ReachabilityFence()->clear_referent(_igvn); // redundant fence
> 
> Thanks for refactoring a bit here :)
> 
> Is this rf guaranteed to belong to the Allocation somehow?

Ah, you could mention that later `ReachabilityFenceNode::Identity` removes the rf.

> src/hotspot/share/opto/reachability.cpp line 136:
> 
>> 134:     return true;
>> 135:   }
>> 136: }
> 
> Nit: `an no-op` -> `a no-op`
> 
> Also: do you need the return value? The only use case does not do anything with it.

You could mention that `Identity` will remove the node later.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2330138204
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2330236031
PR Review Comment: https://git.openjdk.org/jdk/pull/25315#discussion_r2330237394