RFR: 8334571: Extract control dependency rewiring out of PhaseIdealLoop::dominated_by() into separate method

[Christian Hagedorn]

On Thu, 20 Jun 2024 14:24:23 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> This is another patch split off from the full Assertion Predicates fix.
> 
> `PhaseIdealLoop::dominated_by()` only works for an `IfNode` that dominates another `IfNode`. However, at some point, we want to replace the two `IfNodes` that are generated as Template Assertion Predicate for a hoisted range check with a single dedicated `TemplateAssertionPredicateNode`. To still be able to rewire the control dependencies in Loop Predication from the hoisted range check to the new `TemplateAssertionPredicateNode` outside the loop (instead of to the second `IfNode` projection of the Template Assertion Predicate as done today), we can extract the rewiring part out of `dominated_by()` into a separate method `rewire_safe_outputs_to_dominator()` and use that one instead.
> 
> We get the following in Loop Predication:
> - Invariant checks: Still use `dominated_by()` which calls `rewire_safe_outputs_to_dominator()`
> - Range checks: Use `eliminate_hoisted_range_check()` which calls `rewire_safe_outputs_to_dominator()`.
> 
> Thanks,
> Christian

src/hotspot/share/opto/loopPredicate.cpp line 1188:

> 1186:     dominated_by(hoisted_check_predicate_proj, iff, negated);
> 1187: 
> 1188:     C->print_method(PHASE_AFTER_LOOP_PREDICATION_IC, 4, hoisted_check_predicate_proj->in(0));

Additional benefit: This now dumps the graph after eliminating the hoisted check out of the loop. Same for the range checks.

src/hotspot/share/opto/loopPredicate.cpp line 1286:

> 1284:   assert(new_predicate_proj != nullptr, "sanity");
> 1285:   // Success - attach condition (new_predicate_bol) to predicate if
> 1286:   invar.map_ctrl(if_success_proj, new_predicate_proj); // so that invariance test can be appropriate

Moved to invariant checks and range checks separately.

src/hotspot/share/opto/loopPredicate.cpp line 1304:

> 1302:                                                    IfTrueNode* template_assertion_predicate_proj) {
> 1303:   _igvn.replace_input_of(hoisted_check_proj->in(0), 1, _igvn.intcon(1));
> 1304:   rewire_safe_outputs_to_dominator(hoisted_check_proj, template_assertion_predicate_proj, true);

`pin_array_access_nodes = true` since this is called only for range checks now.

src/hotspot/share/opto/loopopts.cpp line 389:

> 387:         }
> 388:         if (!new_loop->_child) {
> 389:           new_loop->_body.push(out);

Renaming only and adding assertion message

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19806#discussion_r1647676810
PR Review Comment: https://git.openjdk.org/jdk/pull/19806#discussion_r1647695458
PR Review Comment: https://git.openjdk.org/jdk/pull/19806#discussion_r1647683248
PR Review Comment: https://git.openjdk.org/jdk/pull/19806#discussion_r1647680798