Integrated: 8282592: C2: assert(false) failed: graph should be schedulable

[Roland Westrelin]

On Wed, 9 Mar 2022 13:42:01 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> 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.

This pull request has now been integrated.

Changeset: 85628a87
Author:    Roland Westrelin <roland at openjdk.org>
URL:       https://git.openjdk.java.net/jdk/commit/85628a871df3fdeec1b422d1c01c222abe45d0a8
Stats:     86 lines in 3 files changed: 86 ins; 0 del; 0 mod

8282592: C2: assert(false) failed: graph should be schedulable

Reviewed-by: chagedorn, thartmann

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

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