RFR: 8280696: C2 compilation hits assert(is_dominator(c, n_ctrl)) failed

[Tobias Hartmann]

We hit an assert when computing early control via `PhaseIdealLoop::compute_early_ctrl` for `1726 AddP` because one of its control inputs `1478 Region` does not dominate current control `1350 Loop` of the AddP:
![Screenshot from 2022-05-18 15-10-24](https://user-images.githubusercontent.com/5312595/169046641-2ee94257-0aae-4ddf-bb5f-49dc19b466b3.png)

I.e., current control of the AddP is incorrect. The problem is that the code in `PhaseIdealLoop::has_local_phi_input` that special cases AddP's only checks control of the Address (and Offset) input, assuming that control of the Base input is consistent.
https://github.com/openjdk/jdk/blob/aa7ccdf44549a52cce9e99f6569097d3343d9ee4/src/hotspot/share/opto/loopopts.cpp#L333-L337
This is not guaranteed though, leading to the AddP ending up with control that is not dominated by control of its base input.

As described below, this only reproduces with a very specific sequence of optimizations triggered by replay compilation with `-XX:+StressIGVN` and a fixed seed. I was not able to extract a regression test.

The fix is to also check control of the Base input when moving the AddP up to a dominating point. For testing purposes, I added an `assert(get_ctrl(m->in(1)) != n_ctrl, "sanity")` without the fix to verify that this change does not affect common cases. It triggers in the failing case but not for any test in tier 1 - 5. In addition, I slightly refactored the code of `PhaseIdealLoop::compute_early_ctrl` and added comments.

Gory details below.

Relevant graph after parsing:

 197  CastPP  ===  1460  60  [[ ... 1724  1725 ]]  #java/io/BufferedReader:NotNull *
 1459  CastPP  ===  1460  60  [[ ...  1724  1725 ]]  #java/io/BufferedReader:NotNull *
 1725  Phi  ===  1478  1459  197  [[ 1726 ]]  #java/io/BufferedReader:NotNull *
 1724  Phi  ===  1478  1459  197  [[ 1726 ]]  #java/io/BufferedReader:NotNull *
 1726  AddP  === _  1724  1725  41  [[ ... ]]   Oop:java/io/BufferedReader:NotNull+24 *

`1724 Phi` is then processed by the following code in `PhiNode::Ideal` that replaces its inputs by a cast of the unique input `1730 CastPP`:

https://github.com/openjdk/jdk/blob/aa7ccdf44549a52cce9e99f6569097d3343d9ee4/src/hotspot/share/opto/cfgnode.cpp#L2013-L2016

 ```
  197  CastPP  ===  1460  60  [[ ...  1725 ]]  #java/io/BufferedReader:NotNull *
 1459  CastPP  ===  1460  60  [[ ...  1725 ]]  #java/io/BufferedReader:NotNull *
 1730  CastPP  ===  1478  60  [[ ...  1724  1724 ]]  #java/io/BufferedReader:NotNull * strong dependency
 1725  Phi  ===  1478  1459  197  [[ 1726 ]]  #java/io/BufferedReader:NotNull *
 1724  Phi  ===  1478  1730  1730  [[ 1726 ]]  #java/io/BufferedReader:NotNull *
 1726  AddP  === _  1724  1725  41  [[ ... ]]   Oop:java/io/BufferedReader:NotNull+24 *
 ```
Then `1724 Phi` is replaced by the unique input `1730  CastPP`:

  197  CastPP  ===  1460  60  [[ ...  1725 ]]  #java/io/BufferedReader:NotNull *
 1459  CastPP  ===  1460  60  [[ ...  1725 ]]  #java/io/BufferedReader:NotNull *
 1725  Phi  ===  1478  1459  197  [[ 1726 ]]  #java/io/BufferedReader:NotNull *
 1730  CastPP  ===  1478  60  [[ ...  1726 ]]  #java/io/BufferedReader:NotNull * strong dependency
 1726  AddP  === _  1730  1725  41  [[ ... ]]   Oop:java/io/BufferedReader:NotNull+24 *

Now `1459 CastPP` is replaced by identical `197 CastPP`:

 197  CastPP  ===  1460  60  [[ ...  1725 ]]  #java/io/BufferedReader:NotNull *
 1725  Phi  ===  1478  197  197  [[ 1726  1739 ]]  #java/io/BufferedReader:NotNull *
 1730  CastPP  ===  1478  60  [[ ...  1726 ]]  #java/io/BufferedReader:NotNull * strong dependency
 1726  AddP  === _  1730  1725  41  [[ ... ]]   Oop:java/io/BufferedReader:NotNull+24 *

Finally, `1725 Phi` is replaced by unique input `197 CastPP` and the AddP ends up with two casts with different control of the same oop for Base and Address:

 197  CastPP  ===  1460  60  [[ ...  1725 ]]  #java/io/BufferedReader:NotNull *
 1730  CastPP  ===  1478  60  [[ ...  1726 ]]  #java/io/BufferedReader:NotNull * strong dependency
 1726  AddP  === _  1730  197  41  [[ ... ]]   Oop:java/io/BufferedReader:NotNull+24 *


Looking at the above transformation, the root cause is really the `1730 CastPP` added by `PhiNode::Ideal` which is not needed and prevents the two casts from being merged. Is it worth filing a follow-up enhancement to fix this?

Thanks,
Tobias

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

Commit messages:
 - 8280696: C2 compilation hits assert(is_dominator(c, n_ctrl)) failed

Changes: https://git.openjdk.java.net/jdk/pull/8770/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8770&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8280696
  Stats: 14 lines in 1 file changed: 4 ins; 3 del; 7 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8770.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8770/head:pull/8770

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