RFR: 8319372: C2 compilation fails with "Bad immediate dominator info"

[Vladimir Kozlov]

On Tue, 28 Nov 2023 10:55:13 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> I included the test case from 8318690 because the cause of the failure
> is similar, the fix I propose applies to both. If this fix goes in
> then 8318690 can be closed as duplicate of this one.
> 
> In both cases, a `CastII`'s type is narrowed because it is control
> dependent on a condition that tests the input of the `CastII` (code in
> `CastIINode::Value()`). In both cases, that `CastII` is input to a
> range check `CastII`. Predication leaves that second `CastII` in the
> loop body but the range check that guards it is replaced by
> predicates. The difference in the 2 test cases is where the first
> `CastII` comes from: `PhiNode::Ideal()` in one case,
> `PhaseIdealLoop::try_sink_out_of_loop()` in the other. The narrowed
> type of the first `CastII` conflicts with the type of the range check
> `CastII` and that causes that one to become top. The path where that
> `CastII` is is unreachable but c2 can't prove it. The inconsistency
> causes the asserts.
> 
> To fix this we would need to prove that the path where the `CastII`
> nodes are is dead and for that we would need to prove that the
> condition that guards that path and at least one of the predicates
> can't be true at the same time. While it may be feasible, that seems
> like a lot of extra complexity.
> 
> Instead I propose removing the logic in `CastIINode::Value()`
> entirely. I went back to the change that introduced it and, as I
> understand, it was added as an attempt to detect and remove useless
> `CastII` nodes. There was no correctness issue involved AFAICT.

Agree.

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

Marked as reviewed by kvn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16844#pullrequestreview-1753466628