RFR: 8342287: C2 fails with "assert(is_IfTrue()) failed: invalid node class: IfFalse" due to Template Assertion Predicate with two UCTs [v2]

[Julian Waters]

On Thu, 17 Oct 2024 21:12:28 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> ### Assertion Predicates Have the True Projection on the Success Path
>> By design, Template and Initialized Assertion Predicates have the true projection on the success path and false projection on the failing path. 
>> 
>> ### Is a Node a Template Assertion Predicate?
>> Template Assertion Predicates can have an uncommon trap on or a `Halt` node following the failing/false projection. When trying to find out if a node is a Template Assertion Predicate, we call `TemplateAssertionPredicate::is_predicate()` which checks if the provided node is the success projection of a Template Assertion Predicate. This involves checking if the other projection has a `Halt` node or an UCT (L141):
>> https://github.com/openjdk/jdk/blob/7a64fbbb9292f4d65a6970206dec1a7d7645046b/src/hotspot/share/opto/predicates.cpp#L135-L144
>> 
>> ### New `PredicateIterator` Class
>> 
>> [JDK-8340786](https://bugs.openjdk.org/browse/JDK-8340786) introduced a new `PredicateIterator` class to simplify iteration over predicates and replaced code that was doing some custom iteration. 
>> 
>> #### Usual Usage
>> Normally, we always start from a loop entry and follow the predicates (i.e. reaching a predicate over its success path).
>> 
>> #### Special Usage
>> However, I also replaced the predicate skipping in `PhaseIdealLoop::build_loop_late_post_work()` with the new `PredicateIterator` class which could start at any node, including a failing path false projection.
>> 
>> ### Problem: Two Uncommon Traps for a Template Assertion Predicate
>> The fuzzer now found a case where a Template Assertion Predicate has a predicate UCT on the failing **and** the success path due to folding away some dead nodes:
>> 
>> ![image](https://github.com/user-attachments/assets/c2a9395e-9eaf-46a0-b978-8847d8e21945)
>> 
>> In the Special Usage mentioned above, we could be using the `PredicateIterator` with `505 IfFalse` as starting node. In the core method `RegularPredicateBlockIterator::for_each()` for the traversal, we call the following with `current` = `505 IfFalse`:
>> https://github.com/openjdk/jdk/blob/1ea1f33f66326804ca2892fe0659a9acb7ee72ae/src/hotspot/share/opto/predicates.hpp#L628-L629
>> `TemplateAssertionPredicate::is_predicate()` succeeds because we have a valid Template Assertion Predicate If node and the other projection `504 IfTrue` has a predicate UCT. We then fail when trying to convert the `IfFalse` to an `IfTrue` with `current->as_IfTrue()` on L629.
>> 
>> ### Solution
>> The fix is straight forward: `TemplateAssertionPr...
>
> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review Vladimir

I might be missing something, but why is the new test inside src/hotspot?

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

PR Comment: https://git.openjdk.org/jdk/pull/21561#issuecomment-2428001222