RFR: 8341977: Replace predicate walking and cloning code for Loop Peeling with a predicate visitor [v4]

[Emanuel Peter]

On Mon, 28 Oct 2024 13:46:42 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> #### Replacing the Remaining Predicate Walking and Cloning Code 
>> In the next series of patches (this, [JDK-8342943](https://bugs.openjdk.org/browse/JDK-8342943), [JDK-8342945](https://bugs.openjdk.org/browse/JDK-8342945), and [JDK-8342946](https://bugs.openjdk.org/browse/JDK-8342946)) I want to replace the predicate walking and cloning code used for Loop Peeling, Pre/Main/Post Loops, Loop Unswitching, removing useless Assertion Predicates and Loop Unrolling with new `PredicateVisitors` which can be used in combination with the new `PredicateIterator`.
>> 
>> #### Single Template Assertion Predicate Check
>> This replacement allows us to have a single `TemplateAssertionPredicate::is_predicate()` check that is called for all predicate matching code. This enables the removal of uncommon traps for Template Assertion Predicates with [JDK-8342047](https://bugs.openjdk.org/browse/JDK-8342047) which is a missing piece in order to fix the remaining problems with Assertion Predicates ([JDK-8288981](https://bugs.openjdk.org/browse/JDK-8288981)).
>> 
>> #### Common Refactorings for all the Patches in this Series
>> In each of the patch, I will do similar refactoring ideas:
>> - Replace the existing code in the corresponding `PhaseIdealLoop` method with call to a new (or existing) predicate visitor which extends the `PredicateVisitor` interface. 
>> - The visitor implements the Assertion Predicate `visit()` methods to implement the cloning and initialization of the Template Assertion Predicates.
>> - The predicate visitor is then passed to the `PredicateIterator` which walks through all predicates found at a loop and applies the visitor for each predicate. 
>> - The visitor creates new nodes (if there are Template Assertion Predicates) either in place or at the loop entry of a target loop. In the latter case, the calling code of the `PredicateIterator` must make sure to connect the tail of the newly created predicate chain after the old loop entry to the target loop head (see **P1** PR comment).
>> - Keep the semantics which includes to only apply the Template Assertion Predicate processing if there are Parse Predicates (see **P2** PR comment). This limitation should eventually be removed. But I want to do that separately at a later point.
>> 
>> #### Refactorings of this Patch
>> This first patch replaces the predicate walking and cloning code for Loop Peeling and lays the foundation for the replacement for main/post loops ([JDK-8342943](https://bugs.openjdk.org/browse/JDK-8342...
>
> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update src/hotspot/share/opto/loopTransform.cpp
>   
>   Co-authored-by: Emanuel Peter <emanuel.peter at oracle.com>

Now it looks good to me :)

Oh, I think you actually have a build failure:

=== Output from failing command(s) repeated here ===
* For target hotspot_variant-server_libjvm_objs_loopTransform.o:
/home/runner/work/jdk/jdk/src/hotspot/share/opto/loopTransform.cpp: In member function ‘void PhaseIdealLoop::create_assertion_predicates_at_loop(CountedLoopNode*, CountedLoopNode*, const NodeInLoopBody&)’:
/home/runner/work/jdk/jdk/src/hotspot/share/opto/loopTransform.cpp:1991:55: error: ‘create_assertion_predicates_for_loop’ was not declared in this scope; did you mean ‘create_assertion_predicates_at_loop’?
 1991 |     IfTrueNode* last_created_predicate_success_proj = create_assertion_predicates_for_loop.last_created_success_proj();
      |                                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                       create_assertion_predicates_at_loop

* All command lines available in /home/runner/work/jdk/jdk/build/linux-x64/make-support/failure-logs.
=== End of repeated output ===

src/hotspot/share/opto/loopTransform.cpp line 1986:

> 1984:   Node* target_loop_entry = target_outer_loop_head->in(LoopNode::EntryControl);
> 1985:   CreateAssertionPredicatesVisitor create_assertion_predicates_visitor(init, stride, target_loop_entry, this,
> 1986:                                                                         _node_in_loop_body);

optional: fix indentation

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

Marked as reviewed by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21679#pullrequestreview-2399278122
PR Comment: https://git.openjdk.org/jdk/pull/21679#issuecomment-2441764541
PR Review Comment: https://git.openjdk.org/jdk/pull/21679#discussion_r1819179462