RFR: 8282592: C2: assert(false) failed: graph should be schedulable [v2]

[Roland Westrelin]

> This is another case of overunrolling that causes the type of the main
> loop's iv to conflict with a ConvI2L of an array access. Skeleton
> predicates were added to solve this family of issues. A predicate that
> should catch that the main loop is unreachable is indeed added but the
> predicate's condition doesn't constant fold.
> 
> What happens is that the main loop's iv phi initial value is a CastII
> (added by PhaseIdealLoop::cast_incr_before_loop()). That CastII is
> input to the predicates above the main loop and of the main loop's iv
> phi. Once loop opts are over, the type of the CastII is updated by
> CastIINode::Value() (the logic that looks at the test that guards the
> CastII). The type update causes the main loop iv phi's type to be
> updated and a ConvI2L to be become top. The predicate that should
> catch the overunrolling has the shape:
> 
> (CmpU (AddI#1 (CastII (AddI#2 (Phi ..) ConI#1)) ConI#2) ConI#3)
> 
> In the case of the test case:
> 
> ConI#3 = 10
> ConI#2 = 7
> ConI#1 = 1
> 
> CastII has type = int:<=max-8
> Phi has type = int:>=8
> 
> As a result:
> AddI#2 has type int
> AddI#1 has type int:min+7..max-1
> 
> The Phi is the pre loop Phi.
> 
> The fix I propose is that predicates when they are updated should skip
> over the CastII, that is in this case:
> 
> (CmpU (AddI#1 (AddI#2 (Phi ..) ConI#1) ConI#2) ConI#3)
> 
> which is reshaped to:
> 
> (CmpU (AddI#1' (Phi ..) ConI#1') ConI#3)
> 
> In this case:
> 
> ConI#1 = 4 (because the predicate constant fold in earlier passes of
> unrolling)
> 
> As a result AddI#1' still has type int But CmpUNode has special logic
> to deal with the (CmpU (AddI ...)) shape and it can determine that
> this one always fails.
> 
> The role of the CastII is to enforce the dependency between nodes in
> the loop and the the loop entry test so it's not required for
> predicates.
> 
> With that change, compiler/loopopts/TestOverunrolling.java fails
> because a predicate doesn't constant fold anymore. The reason in that
> case, is that PhaseIdealLoop::reorg_offsets() adds an Opaque2 between
> the pre loop iv phi and the predicate that hides the type of the iv
> phi. Without the change, the CastII captures the type of the iv phi
> and the Opaque2 node is added between the CastII and the pre loop iv
> phi. To solve that problem, I propose that
> PhaseIdealLoop::reorg_offsets() always add a CastII after the Opaque2
> so the type of the iv phi is not lost.

Roland Westrelin has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:

 - test run failure
 - Merge branch 'master' into JDK-8282592
 - test & fix

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7758/files
  - new: https://git.openjdk.java.net/jdk/pull/7758/files/ef7344d1..f0ca52d7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7758&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7758&range=00-01

  Stats: 7412 lines in 314 files changed: 3737 ins; 1955 del; 1720 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7758.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7758/head:pull/7758

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