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

[Vladimir Kozlov]

On Wed, 29 Nov 2023 13:25:30 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.
>
> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review

Update is good.

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

Marked as reviewed by kvn (Reviewer).

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