RFR: 8260420: C2 compilation fails with assert(found_sfpt) failed: no node in loop that's not input to safepoint

[Tobias Hartmann]

Loop strip mining verification fails because a `LoadNode` with no safepoint use ends up in the `OuterStripMinedLoop`. The root cause is very similar to [JDK-8249607](https://bugs.openjdk.java.net/browse/JDK-8249607): A `LoadNode` with two uses (a field store and a return, see `test2`) is cloned by `PhaseIdealLoop::split_if_with_blocks_post()` for each use, to allow it to flow out of the loop. Both clones end up in the `OuterStripMinedLoop`, the clone for the field store because the store has a safepoint use and the clone for the return because `late_load_ctrl` is too conservative by taking all initial uses into account.

Now the load without a safepoint use is always detected by loop strip mining verification but without verification (for example, in a product build), we still hit several different issues depending on the exact use of the load (compare `test2/3/4`). The main issue is that loads without a safepoint use are not correctly wired when creating pre/main/post loops. Christian described this well in his RFR for [JDK-8249607](https://bugs.openjdk.java.net/browse/JDK-8249607): https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-August/039638.html

Therefore, relaxing loop strip mining verification is not an option.

The problem with the fix for [JDK-8249607](https://bugs.openjdk.java.net/browse/JDK-8249607) is that it relies on IGVN being executed before pre/main/post loops are created, merging the two clones such that the remaining one has a safepoint use and is therefore correctly handled. However, this is not guaranteed. In fact, if `major_progress` is not set, pre/main/post loops could be created in the same round of loop opts without IGVN in-between (`IdealLoopTree::iteration_split` is executed right after `PhaseIdealLoop::split_if_with_blocks`).

I think we have the following options:
1) Bail out if the loop is strip mined. I think that would be too conservative.

2) Simply set `major_progress` when pinning a `LoadNode` to the `OuterStripMinedLoop` to make sure IGVN is executed and duplicate `LoadNodes` are merged. That seems quite invasive though.

3) Allow the cloned load without a safepoint use (and therefore no usages in the `OuterStripMinedLoop`) to completely flow out of the loop. Currently, this is blocked by `late_load_ctrl` being computed on the initial `LoadNode` instead of the clone and therefore also taking into account the store that is referenced by the safepoint. We could simply re-compute the late ctrl for the cloned load, allowing it to flow out of the `OuterStripMinedLoop`. However, that only works if there is no anti-dependency in the `OuterStripMinedLoop`.

4) Detect duplicate loads in the `OuterStripMinedLoop` on creation in `PhaseIdealLoop::split_if_with_blocks` and merge them right away.

I've decided to go with a combination of 3) and 4). If there is not anti-dependency, 3) will ensure that the load is moved out of the `OuterStripMinedLoop`. If there is an anti-dependency, 4) will ensure that still no duplicate loads are created.

I've also reverted the fix for JDK-8249607 which is no longer required.

As Roland and Christian already noticed, there are various other issues with that code that hopefully will be addressed at some point by [JDK-8252372](https://bugs.openjdk.java.net/browse/JDK-8252372).

Thanks,
Tobias

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

Commit messages:
 - 8260420: C2 compilation fails with assert(found_sfpt) failed: no node in loop that's not input to safepoint

Changes: https://git.openjdk.java.net/jdk/pull/2315/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2315&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260420
  Stats: 94 lines in 2 files changed: 77 ins; 1 del; 16 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2315.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2315/head:pull/2315

PR: https://git.openjdk.java.net/jdk/pull/2315