[jdk17] RFR: 8268883: C2: assert(false) failed: unscheduable graph

[Christian Hagedorn]

On Fri, 2 Jul 2021 09:12:13 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> The IR graph has a diamond shaped:
> (If (Bool (CmpI ...)))/Region/Phi.
> 
> One of the Phi's input is a chain of arithmetic operations. The last
> of the chain is a CastII in one branch of the If. The input of the
> CastII is also the input of a CmpI node that's input to the If. The
> other input of the CmpI becomes a constant. That causes the CastII's
> type be narrower because of logic in CastIINode::Value(). The improved
> type percolates through the chain of arithmetic operations. The type
> of a ConvL2I in the chain is updated to a narrower type as a
> consequence. Now the other input of the CmpI becomes a constant and as
> a consequence the branch of the diamond where the arithmetic
> operations are is never taken. What would next happen, is the ConvL2I
> becomes top (because its narrow type and its input type do not
> overlap), one of the Phi inputs would become top and the Phi would be
> replaced by the remaining input. The If would constant fold and only
> one branch would be kept. But before that has a chance to happen,
> PhiNode::Ideal() runs and is_cond_add() transforms it into a:
> 
> (AddI (AndI ..))
> 
> with one input that's the ConvL2I. When that input becomes top, the
> AndI becomes top and the AddI that replaced the Phi too. So indirectly
> the Phi is replaced by top which shouldn't have happened.
> 
> The fix I propose is to delay the is_cond_add() (and similar
> transformations) until the CmpI has a chance to be transformed. I
> think it's enough to fix this issue because:
> 
> - As long as one of the If projections has a use other than the
>   Region/Phi (such as the CastII), is_cond_add() is not applied
> 
> - the CastII is the root of the problem because it's what narrows the
>   type so much that some node becomes top. The CastII can only be
>   removed once its input becomes constant and in that case the CmpI's
>   input is also a constant and the CmpI is enqueued for IGVN

That looks reasonable.

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

Marked as reviewed by chagedorn (Reviewer).

PR: https://git.openjdk.java.net/jdk17/pull/201