RFR: 8314106: C2: assert(is_valid()) failed: must be valid after JDK-8305636

[Christian Hagedorn]

On Thu, 10 Aug 2023 13:35:46 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> In the failing test case, we are unswitching a loop for which we've already removed Parse Predicates with `Compile::cleanup_parse_predicates()`. We are wrongly checking if a predicate block is non-empty (i.e. find the Parse **or** Runtime Predicates) instead of only checking if we find the Parse Predicate:
> https://github.com/openjdk/jdk/blob/23fe2ece586d3ed750e905e1b71a2cd1da91f335/src/hotspot/share/opto/loopPredicate.cpp#L448-L453
> 
> In the test case, we have a predicate block that contains Runtime Predicates from Loop Predication but no Parse Predicate anymore. Therefore, when trying to clone the non-existing Parse Predicate, we fail with the assertion because we do not have a valid Parse Predicate.
> 
> The fix is to only clone a Parse Predicate and the Assertion Predicates for a predicate block if the Parse Predicate is actually there. This is not entirely correct because we would also need to clone Assertion Predicates in the absence of Parse Predicates. But this was already wrong before JDK-8305636:
> https://github.com/openjdk/jdk/blob/a38fdaf18dfeeb23775516d1986c720190ba9fc2/src/hotspot/share/opto/loopPredicate.cpp#L598-L612
> 
> This will only be fixed with the complete fix ([JDK-8288981](https://bugs.openjdk.org/browse/JDK-8288981)). The proposed fix here just reverts back to the old behavior before JDK-8305636.
> 
> Thanks,
> Christian

Thanks Tobias and Vladimir for your reviews!

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

PR Comment: https://git.openjdk.org/jdk/pull/15225#issuecomment-1674374719