RFR: 8335393: C2: assert(!had_error) failed: bad dominance

[Roland Westrelin]

In `test1/test1Helper`:


v = array[i - 1];


is only used out of loop so it's sunk out of loop. Next pre/main/post
loops are created and range check elimination takes care of removing
the range check from the main loop. The main loop is then empty so
it's removed. The array access which is now out of loop becomes:


v = array[-2]


-1 is the final iteration of the main loop. -2 is obviously out of
bound and causes the range check `CastII` for the array access to
become top and the graph to be broken.

The main loop is never entered after range check elimination but the
zero trip guard for the main loop doesn't constant fold. That happens
because the init value of the loop is unkown which causes the loop
`Phi` of the pre loop (and main/post loops as well) to be of type
`int`.

For this bug to occur, I think we need an unknown value for the init
value of the loop. That way the zero trip guard doesn't constant
fold. We also need a known final loop value which happens when the
loop limit is known and the final loop only depends on the loop
limit. That can only happen when the stride of the loop is 1 (or -1).

If the range check is eliminated by predication, then a predicate for
the first and last loop variable values are added. This would catch
the case where the array access is out of bound for the last loop
variable value. So the bug can only occur with range check
elimination.

I see 2 ways to fix this. One is to narrow the type of the loop
Phi. After all we known that init < limit so in the test case that
start < 0. The other way to fix this is to add an assert predicate for
the final value of the loop variable every time an range check is
eliminated by range check elimination. That assert predicate only
needs to be added by range check elimination when the stride is 1 or
-1. It doesn't need to be updated when unrolling happens (because then
the stride can't be 1 or -1). That makes for a reasonably simple fix.

The reason I would rather go with the assert predicate fix is that
changing type computation of the iv phi node will affect a lot more
code and it's hard to be confident that it won't reveal new bugs.

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

Commit messages:
 - fix & test

Changes: https://git.openjdk.org/jdk/pull/20140/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20140&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8335393
  Stats: 102 lines in 2 files changed: 100 ins; 2 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/20140.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/20140/head:pull/20140

PR: https://git.openjdk.org/jdk/pull/20140