RFR: 8296318: use-def assert: special case undetected loops nested in infinite loops

[Emanuel Peter]

On Tue, 13 Dec 2022 09:56:26 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> `PhaseCFG::verify` checks that the nodes scheduled into the blocks have "use-after-def".
>> https://github.com/openjdk/jdk/blob/8c472e481676ed0ef475c4989477d5714880c59e/src/hotspot/share/opto/block.cpp#L1376
>> 
>> As long as the control flow has no loops, this should always hold.
>> We make sure to not check the assert if the block-head is a `LoopNode`, and the use `n` is a `Phi`, since we may have inputs from the backedge, which would be "use-before-def" in the block-scheduling order, but that is to be expected.
>> https://github.com/openjdk/jdk/blob/8c472e481676ed0ef475c4989477d5714880c59e/src/hotspot/share/opto/block.cpp#L1364
>> 
>> **Problem**
>> This assumes that all loops are properly detected, if a loop was not detected the block-head may only be a `RegionNode`, and the assert can fail (see regression test). Why are not all loops detected?
>> During `PhaseIdealLoop::build_loop_tree`, we detect all loops, but not always attach infinite loops with their sub-loops to the loop tree, and then the loop-head `Regions` are not converted to `LoopNodes` in `PhaseIdealLoop::beautify_loops`.
>> This behaviour is expected, see also https://github.com/openjdk/jdk/pull/11473.
>> Thus, the assert fires, but it should not.
>> 
>> **Solution**
>> Add special casing to assert: also accept if the `Region` is in an infinite subgraph, and we are looking at a `Phi` as use (the def could be values from the backedge).
>
> src/hotspot/share/opto/block.cpp line 1383:
> 
>> 1381:             assert(block->find_node(def) < j ||
>> 1382:                    is_loop ||
>> 1383:                    (block->head()->as_Region()->is_in_infinite_subgraph() && n->is_Phi()),
> 
> I suggest to swap the order of `n->is_Phi()` and `block->head()->as_Region()->is_in_infinite_subgraph()`. But this should be a rare case anyways, so it does not really matter that much.

👍

> src/hotspot/share/opto/cfgnode.cpp line 409:
> 
>> 407: // (no path to root except through false NeverBranch exit)
>> 408: // worklist is directly used for the traversal
>> 409: bool RegionNode::are_all_in_infinite_subgraph(Unique_Node_List &worklist) {
> 
> Suggestion:
> 
> bool RegionNode::are_all_nodes_in_infinite_subgraph(Unique_Node_List& worklist) {

👍

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

PR: https://git.openjdk.org/jdk/pull/11642