RFR: 8327111: Replace remaining usage of create_bool_from_template_assertion_predicate() which requires additional OpaqueLoop*Nodes transformation strategies [v2]

[Emanuel Peter]

On Fri, 5 Apr 2024 14:56:20 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> https://github.com/openjdk/jdk/pull/18293 started to replace `create_bool_from_template_assertion_predicate()` usages to fix an endless DFS traversal problem. This patch is a follow-up to replace the last usage of `create_bool_from_template_assertion_predicate()` in `clone_assertion_predicate_and_initialize()` to completely fix the problem.
>> 
>> Depending on where `clone_assertion_predicate_and_initialize()` is called from, we need to clone the Template Assertion Predicate Expression differently:
>> - Create a new Template Assertion Predicate for a main loop: Clone everything except for the `OpaqueLoopInitNode` which needs to be replaced with a new `OpaqueLoopInitNode` (done with `clone_and_replace_init()`).
>> - Create an Initialized Assertion Predicate for all other cases: Clone everything except for the `OpaqueLoop*Nodes` which are replaced with an actual init and stride value (done with `clone_and_replace_init_and_stride()`).
>> 
>> Note that it's incorrect to _not_ create new Template Assertion Predicates in case we split the loop further (for example after peeling a loop). This will be eventually fixed with [JDK-8288981](https://bugs.openjdk.org/browse/JDK-8288981).
>> 
>> I've extended the test added with https://github.com/openjdk/jdk/pull/18293 to also cover the now fixed cases. 
>> 
>> Thanks,
>> Christian
>
> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove othervm

Looks good, a nice continuation of your series of fixes and refactorings!

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

> 1497:   Opaque4Node* new_opaque4_node;
> 1498:   if (new_stride == nullptr) {
> 1499:     new_opaque4_node = template_assertion_predicate_expression.clone_and_replace_init(new_init, control, this);

Optional: Maybe you could add a small comment about why the `new_stride` is a `nullptr` in this case?

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

Marked as reviewed by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18628#pullrequestreview-1983565016
PR Review Comment: https://git.openjdk.org/jdk/pull/18628#discussion_r1553901435