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

[Christian Hagedorn]

On Mon, 28 Aug 2023 15:18:03 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> 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?

When creating a new `ParsePredicateNode`, we are registering it for post loop opts IGVN:
https://github.com/openjdk/jdk/blob/d50356e14877b4d4376fc18ecdec29f7d98d77bd/src/hotspot/share/opto/ifnode.cpp#L1978-L1984

During post loop opts IGVN (or earlier if `_useless` is true), the `ParsePredicateNode` is folded when calling `ParsePredicateNode::Value()`:
https://github.com/openjdk/jdk/blob/d50356e14877b4d4376fc18ecdec29f7d98d77bd/src/hotspot/share/opto/ifnode.cpp#L2004-L2011

I don't think that _useful_ `ParsePredicateNodes` should ever be cloned. If a loop is removed, then either major progress is true and `eliminate_useless_predicates()` will mark them useless (i.e. set `_useless` to true) in the next round of loop opts or if major progress was false then `Compile::mark_parse_predicate_nodes_useless()` have already marked them useless. In both cases, they will be removed by the next round of IGVN. Otherwise, I don't think that useful `ParsePredicateNodes` should ever be split (i.e. being part of a loop body to be cloned) or cloned otherwise (i.e. by split-if etc.). When cloning them to unswitched loops, I create them newly by calling the constructor, so they end up on the post loop opts IGVN list:
https://github.com/openjdk/jdk/blob/d50356e14877b4d4376fc18ecdec29f7d98d77bd/src/hotspot/share/opto/loopPredicate.cpp#L310-L311

However, there are some cases where useless `ParsePredicateNodes` are cloned. For example, parsing could have added them to Ifs inside a loop. But then `eliminate_useless_predicates()` will mark them useless. When splitting the loop and cloning these `ParsePredicateNodes`, they will still be marked useless and they are all cleaned up during the next round of IGVN. So, I think registering `ParsePredicateNodes` for post loop opts once in the constructor should be enough.

Nevertheless, I still register them in the `Compile::_parse_predicates` list in `Node:clone()` to not miss this:
https://github.com/openjdk/jdk/blob/d50356e14877b4d4376fc18ecdec29f7d98d77bd/src/hotspot/share/opto/node.cpp#L511-L513

After post loop opts IGVN, I assert that we got rid of all the `ParsePredicateNodes`:
https://github.com/openjdk/jdk/blob/d50356e14877b4d4376fc18ecdec29f7d98d77bd/src/hotspot/share/opto/compile.cpp#L1856-L1858

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

PR Comment: https://git.openjdk.org/jdk/pull/15449#issuecomment-1697016855