RFR: 8333252: C2: assert(assertion_predicate_has_loop_opaque_node(iff)) failed: must find OpaqueLoop* nodes

Christian Hagedorn chagedorn at openjdk.org
Fri May 31 13:01:13 UTC 2024


[JDK-8330386](https://bugs.openjdk.org/browse/JDK-8330386) added some additional asserts to ensure that we are dealing with Template Assertion Predicates and not non-null-checks which both use `Opaque4` nodes.

#### Correct Assertion
One of this assert was now hit with a fuzzer found case in `get_assertion_predicates()` called during the elimination of useless predicates. We walk through all loops and collect all useful Template Assertion Predicates and Parse Predicates above the loops. For that we look at the UCTs which are shared among the predicates. When finding a predicate with such an UCT which also has an `Opaque4` node, we know that it is a Template Assertion Predicate. We additionally assert that we must find the `OpaqueLoop*Nodes` above which always belong to a template:

https://github.com/openjdk/jdk/blob/7ab74c5f268dac82bbd36355acf8e4f3d357134c/src/hotspot/share/opto/loopPredicate.cpp#L346-L354

So, this assert looks correct.  

#### Why didn't we find `OpaqueLoop*Nodes` in this case?
For the Template Assertion Predicate for the last value, we insert an additional `CastII` to keep the type information of the iv phi: 

https://github.com/openjdk/jdk/blob/7ab74c5f268dac82bbd36355acf8e4f3d357134c/src/hotspot/share/opto/loopPredicate.cpp#L1323-L1324

But in the test case, the type of the iv phi is a constant (`521 CastII`):

![image](https://github.com/openjdk/jdk/assets/17833009/5dc17b9c-abfe-4846-89a1-4e189234b991)

`521 CastII` will simply be replaced with a constant during IGVN and the `OpaqueLoop*Nodes` above are removed. We therefore cannot find them anymore later when trying to eliminate useless predicates and we hit the assert.

#### Why does the `CastII`/iv phi have a constant type?
Having a constant type for the iv phi indicates that the counted loop is only going to be executed for one iteration. But C2 has not had the chance, yet, to fold the loop exit test to remove the loop.

#### How to fix this bug?
Having a single iteration loop raises the question, why we even bother to try and hoist checks out of such a loop with Loop Predication in the first place. I therefore suggest to simply bail out of Loop Predication if the trip count is 1. This will also prevent us from creating a Template Assertion Predicate with a `CastII` with a constant type from the iv phi which would be folded.

To do that, we can compute the trip count on entry of Loop Predication. By doing that, we can also remove the trip count computation added for hoisting range checks with [JDK-8267928](https://bugs.openjdk.org/browse/JDK-8267928):

https://github.com/openjdk/jdk/blob/7ab74c5f268dac82bbd36355acf8e4f3d357134c/src/hotspot/share/opto/loopPredicate.cpp#L1212-L1215

This is no longer necessary now. I've added an assert instead to ensure that we've indeed have the correct trip counter computed at Loop Predication entry (might be overly cautious but it's easy to do and a small overhead). I've also added an assert when creating Template Assertion Predicates that we are not having a constant typed iv phi.

Thanks,
Christian

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

Commit messages:
 - 8333252: C2: assert(assertion_predicate_has_loop_opaque_node(iff)) failed: must find OpaqueLoop* nodes

Changes: https://git.openjdk.org/jdk/pull/19500/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19500&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8333252
  Stats: 77 lines in 2 files changed: 74 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/19500.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19500/head:pull/19500

PR: https://git.openjdk.org/jdk/pull/19500


More information about the hotspot-compiler-dev mailing list