[jdk18] RFR: 8278420: C2: assert(!n->is_Store() && !n->is_LoadStore()) failed: no node with a side effect

[Christian Hagedorn]

On Fri, 10 Dec 2021 15:43:52 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> The test case fails with the assertion when an actual unreachable store node with only uses outside of the loop is tried to be sunk out of a dead loop in split-if. This is quite an edge case in which C2 is not able to remove the inner loop but the store for `iFldArr2` inside this loop dies due to improved type information after peeling. This removes some memory phis as well and leaves the store `iFldArr1` with only outside the loop uses. A more detailed explanation how we end up in this situation is shown in the comments of the test case.
> 
> This suggests that the assertion is too strong. I propose to relax the assertion and bail out if we are trying to sink a store node. However, I don't think that we will reach this code with `LoadStore` nodes as they have other memory outputs inside a loop, preventing to reach this assertion code.
> 
> Thanks,
> Christian

After digging deeper into this, I found more issues with loop peeling and loop predication by writing additional tests. I basically found all the issues related to skeleton predicates again but this time with an additional peeling before which makes it quite rare and hard to trigger in practice. I think we need the full range of fixes between the loop and the peeled iteration that we have done before:
1. Initialized skeleton predicate as suggested above to remove a dead loop after peeling.
2. Initial skeleton predicates for the initial value and one for the changing stride with unrolling. We could create pre/main/post and then over-unroll the main loop which lets `CastII` nodes die while the main loop is not removed ([JDK-8193130](https://bugs.openjdk.java.net/browse/JDK-8193130), [JDK-8203915](https://bugs.openjdk.java.net/browse/JDK-8203915)).
3. Update data nodes dependent on the predicates before the peeled iteration down to the loop. Because when later unrolling the loop, the cloned data nodes get again the same control input to the predicates before the peeled iteration. This could schedule loads from the main loop even though the main loop is never entered ([JDK-8237859](https://bugs.openjdk.java.net/browse/JDK-8237859) but with peeling before).

In addition, we could also think about adding empty predicates between the loop and the peeled iteration to enable more rounds of loop predication similar to what we are doing for long counted loops. But this is more an idea for an RFE.

There is currently no easy workaround for 1. - 3. and fixing them completely with skeleton predicates is more complex. I don't think it is worth the risk to do this in 18 given how rare 1. - 3. are. On top of that, these are not recent regressions. I will therefore close this PR and re-target this bug to 19/mainline.

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

PR: https://git.openjdk.java.net/jdk18/pull/11