RFR: 8333644: C2: assert(is_Bool()) failed: invalid node class: Phi [v2]

[Vladimir Kozlov]

On Wed, 5 Jun 2024 14:48:12 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> In the patch of [JDK-8330386](https://bugs.openjdk.org/browse/JDK-8330386), I've added a test case that took the code path in `clone_loop_handle_data_uses()` with the new `OpaqueInitializedAssertionPredicateNode`. So, it proved that we should also add a case for `OpaqueInitializedAssertionPredicate` - yet somehow, I forgot to do that. Unfortunately, I could not come up with a failing test if we do not handle this case separately and the mistake went unnoticed.
>> 
>> But the fuzzer has now found such a case where we crash when creating the Mach graph because we have an `If` with a `Phi` input instead of a `Bool`. This happens exactly because of not handling `OpaqueInitializedAssertionPredicate` in  `clone_loop_handle_data_uses()`. 
>> 
>> We have an outside use of a `4843 Bool` (`4193 RangeCheck`) which is also an input into a `OpaqueInitializedAssertionPredicate` node:
>> 
>> ![image](https://github.com/openjdk/jdk/assets/17833009/7f43d987-6e29-449e-8392-58e76e111e80)
>> 
>> We should now clone this `Bool/Cmp` down to avoid having a `Phi` between the `OpaqueInitializedAssertionPredicate` and
>> the `Bool` node which currently wrongly happens:
>> 
>> ![image](https://github.com/openjdk/jdk/assets/17833009/a10554a0-d48a-406c-a510-fef9666c45af)
>> 
>> and we crash later. With this simple patch, this is being avoided.
>> 
>> The good thing is that we now have a test that makes sure that this new condition is properly tested.
>> 
>> Thanks,
>> Christian
>
> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update test/hotspot/jtreg/compiler/predicates/assertion/TestOpaqueInitializedAssertionPredicateNode.java
>   
>   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>

Looks good.

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

Marked as reviewed by kvn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19561#pullrequestreview-2099706997