RFR: 8305637: Remove Opaque1 nodes for Parse Predicates and clean up useless predicate elimination

[Roland Westrelin]

On Mon, 28 Aug 2023 14:38:34 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> This is the last clean-up PR before the complete fix for Assertion Predicates ([JDK-8288981](https://bugs.openjdk.org/browse/JDK-8288981)). 
> 
> This patch includes:
> - Removal of `ConI`->`Opaque1`->`Conv2B` input nodes for `ParsePredicateNodes` with the following additional changes:
>   - Adjusting `ParsePredicateNode` to block unwanted optimizations (added empty `ParsePredicateNode::Ideal()`).
>   - Changing `Compile::_parse_predicate_opaqs` to not store `Opaque1Nodes` to keep track of Parse Predicates but instead storing `ParsePredicateNodes` directly. Renamed to `Compile::_parse_predicates` and adjusted related methods.
>   - Removed asserts matching `Opaque1` -> `Conv2B` shape.
> - Cleaning up `eliminate_useless_predicates()`:
>   - Adjust code to find useful/useless Parse Predicates with the new `Compile::_parse_predicates` list with `ParsePredicateNodes` instead of `Opaque1Nodes`.
>   - Changing `ParsePredicateNode` to carry a `_useless` state which simplifies the elimination of useless predicates with `eliminate_useless_predicates()` and during IGVN (added `ParsePredicateNode::Value()` for that which also removes the predicate once we are in post loop opts IGVN). 
> - Some refactoring/clean-ups of involved code.
>   
> Testing: tier1-7 + some fuzzer testing
> 
> Thanks,
> Christian

Opaque1 nodes fold away after loop opts which guarantees the parse predicate are removed too after loop opts. In the new code, without the Opaque1 nodes, what causes the parse predicate to be removed after loop opts?

src/hotspot/share/opto/loopPredicate.cpp line 314:

> 312:   assert(new_predicate_proj->is_IfTrue(), "the success projection of a Parse Predicate is a true projection");
> 313:   ParsePredicateNode* parse_predicate = new_predicate_proj->in(0)->as_ParsePredicate();
> 314:   _igvn.hash_delete(parse_predicate);

That looks strange. Wasn't the reason for the `hash_delete` in the previous version of the code that the `iff` was then modified. Is it still needed?

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

PR Review: https://git.openjdk.org/jdk/pull/15449#pullrequestreview-1598472992
PR Review Comment: https://git.openjdk.org/jdk/pull/15449#discussion_r1307538018