RFR: 8325494: C2: Broken graph after not skipping CastII node anymore for Assertion Predicates after JDK-8309902 [v2]

[Christian Hagedorn]

On Thu, 11 Apr 2024 08:25:56 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> After range check elimination, a cast in the main loop becomes top
>> because the type of its input (that depends on the iv phi) and the
>> type recorded in the cast do not intersect. This is a case that's
>> expected to be caught by assert predicates but, in this particular
>> case, no assert predicate constant folds.
>> 
>> The stride for the loop is -2. The iv phi type is `min+1..0`
>> 
>> As a consequence, the init value for the main loop has type int.
>> 
>> The range check that causes the issue is for array access:
>> 
>> lArrFld[i11 + 1] = 6;
>> 
>> 
>> The main loop is unrolled once. The second access in the loop is at
>> `i11 - 1` which has type `min..-1`. The range check cast at that
>> access becomes top. The assert predicates operates on an init value
>> that has the shape:
>> 
>> 
>> (CastII (AddI pre_loop_iv -2) int)
>> 
>> 
>> and type int.
>> 
>> That `CastII` is inserted by `PhaseIdealLoop::cast_incr_before_loop()`.
>> 
>> The assert predicate for the first iteration in the main loop is for
>> index:
>> 
>> 
>> (AddI (CastII (AddI pre_loop_iv -2) int) 1)
>> 
>> 
>> And for the second:
>> 
>> 
>> (AddI (CastII (AddI pre_loop_iv -2) int) -1)
>> 
>> 
>> Both have type int so the assert predicate can't constant fold.
>> 
>> I initially fixed this by changing the type of the cast from int to
>> the type of the iv phi:
>> 
>> 
>> (AddI (CastII (AddI pre_loop_iv -2) min+1..0) -1)
>> 
>> 
>> That allows the assert predicate for the second iteration to constant
>> fold. But I was then worried narrowing the type of the cast would
>> causes issues going forward so instead, I propose proceeding as in
>> 8282592 and have assert predicates skip over the CastII (that part of
>> 8282592 was later undone):
>> 
>> 
>> (AddI (AddI pre_loop_iv -2) 1)
>> 
>> 
>> which allows the assert predicate for the first iteration in the main
>> loop to constant fold.
>> 
>> The change from 8282592 caused issues because we used to narrow the
>> type of a cast based on the condition that guards it. That was removed
>> by 8319372.
>
> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review

Thanks for the update, that looks good!

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18724#pullrequestreview-1993609601