RFR: 8317723: C2: CountedLoopEndNodes and Zero Trip Guards are wrongly treated as Runtime Predicate

[Tobias Hartmann]

On Fri, 10 Nov 2023 08:53:29 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> In `testCountedLoopEndAsRuntimePredicate()`, a `CountedLoopEndNode` is wrongly treated as Runtime Predicate and added to a Predicate Block. This happens because a predicate trap ends up at one of the projections of the `CountedLoopEndNode` due to folding some other nodes:
> 
> ![image](https://github.com/openjdk/jdk/assets/17833009/d00af4aa-e962-454f-b7cd-5984dfda293f)
> 
> The verification code then reports this failure when trying to find all predicates above `530 CountedLoop`. I've fixed `RuntimePredicate::is_success_proj()` to only treat an `IfNode` or a `RangeCheckNode` as Runtime Predicate.
> 
> We could also end up treating a zero trip guard If node with an `OpaqueZeroTripGuardNode` as Runtime Predicate (see `testZeroTripGuardAsRuntimePredicate()`):
> 
> ![image](https://github.com/openjdk/jdk/assets/17833009/a515bdfb-b599-4a81-902c-e85b980f5899)
> 
> This could also potentially lead to problems. I've improved `RuntimePredicate::is_success_proj()` to additionally exclude zero trip guard checks. I've strengthened the verification code accordingly such that it would have failed with  `testZeroTripGuardAsRuntimePredicate()`.
> 
> Thanks,
> Christian

Looks good to me otherwise.

src/hotspot/share/opto/predicates.cpp line 99:

> 97: }
> 98: 
> 99: bool RuntimePredicate::is_not_zero_trip_guard(const IfProjNode* if_proj) {

I don't think it's worth adding a new method for this, especially since we should prefer negation of the result in the caller instead of using negation in method/variable names.

test/hotspot/jtreg/compiler/predicates/TestWrongRuntimePredicateDetection.java line 29:

> 27:  * @bug 8317723
> 28:  * @library /test/lib
> 29:  * @summary Test that CountedLoopEndNodes and zero trip guard check If nodes are not treated as Runtime Predicates.

Suggestion:

 * @summary Test that CountedLoopEndNodes and zero trip guards check If nodes are not treated as Runtime Predicates.

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

Marked as reviewed by thartmann (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16596#pullrequestreview-1726719581
PR Review Comment: https://git.openjdk.org/jdk/pull/16596#discussion_r1390738838
PR Review Comment: https://git.openjdk.org/jdk/pull/16596#discussion_r1390734706