RFR: 8333334: C2: Make result of `Node::dominates` more precise to enhance scalar replacement [v7]

[Christian Hagedorn]

On Mon, 19 Aug 2024 09:16:01 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Qizheng Xing has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add copyright.
>
> src/hotspot/share/opto/memnode.cpp line 431:
> 
>> 429: // control input of a memory operation predates (dominates)
>> 430: // an allocation it wants to look past.
>> 431: Node::DomResult MemNode::all_controls_dominate(Node* dom, Node* sub) {
> 
> Now you have many checks of the form:
> 
> all_controls_dominate(this, st_alloc) == DomResult::Dominate
> 
> But actually, there is only one place in IGVN where you care about the third dead code result. Maybe you can abstract that away and do the following:
> - Rename this method to `maybe_all_control_dominate()`.
> - Add a new method `all_control_dominate()` which checks the result for `DomResult::Dominate`:
> 
> bool MemNode::all_controls_dominate(Node* dom, Node* sub) {
>   DomResult dom_result = maybe_all_controls_dominate(dom, sub);
>   return dom_result == DomResult::Dominate 
> }
> 
> - The calls in `LoadNode::split_through_phi()` use `maybe_all_controls_dominate()`.
> - All other callers in existing code do not need to be updated since they call the new `all_controls_dominate()` method which mimics the old behavior without caring about dead code.
> 
> Might be cleaner but it's just a thought.

Maybe also add a method comment about the implications of returning `DomResult::EncounteredDeadCode` (i.e. that this means, we are undecided as long as there is dead code but that at the end of IGVN, we know the definite result once the dead code is cleaned up).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19496#discussion_r1721474671