RFR: 8349139: C2: Div looses dependency on condition that guarantees divisor not null in counted loop [v2]

[Roland Westrelin]

On Tue, 25 Feb 2025 08:07:27 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Roland Westrelin has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
>> 
>>  - Merge branch 'master' into JDK-8349139
>>  - fix & test
>
> src/hotspot/share/opto/loopTransform.cpp line 1703:
> 
>> 1701:   }
>> 1702:   // CastII for the new post loop:
>> 1703:   cast_incr_before_loop(zer_opaq->in(1), zer_taken, post_head);
> 
> I see it is added for the main and post loop. Why not for the pre loop?

The type of the trip `phi` of the loop before pre/main/post is computed from the type of the init value (input 1 of the trip `phi`) and the limit input to the exit condition. The way the trip `phi` type is computed, it can't be narrower than the init value type. If the loop body has a `Div` node and its control dependency is cleared because the divisor is known not zero from the type of the trip `phi`, then the divisor computed from the init value is also not zero and if the backedge of the loop is removed, the `Div` node can float.

i.e.

for (int i = start; i < stop; i += stride) {
  v = x / i;
}

if `i` not zero, it's because its type is computed from `start` and `start` is not zero so, if, for some reason, the loop runs for a single iteration:


v = x/start;


can float as high as the control of `start`.

Same reasoning applies to the pre loop because the trip `phi` of that loop "includes" the whole type of the init value. But that doesn't work for the main (or post loop) because its trip `phi` gets its type from the loop before transformation and it captures the type of the init value for what is now the pre loop which is not the same as the type of the init value for the main (or post loop).

In the example above, let's say `start` has type `[min, -1]` , `stop` is 0 and `stride` is 1. `i`'s type is `[min, -1]`. That doesn't include 0 so the `Div` is free to float. Now once pre/main/post loops are created, the type of the trip `phi` for the main loop is `[min, -1]`.  Now let's say the main loop looses its back edge. The `Div` in the main loop can float and say the value for `i` out of the pre loop is 0. Now the floating `Div` from the main loop can trigger a fault.

> src/hotspot/share/opto/loopnode.cpp line 6091:
> 
>> 6089:   if (uncast && init->is_CastII()) {
>> 6090:     // skip over the cast added by PhaseIdealLoop::cast_incr_before_loop() when pre/post/main loops are created because
>> 6091:     // it can get in the way of type propagation
> 
> I think it would be nice if you said more about how it can get in the way of type propagation. Why would we sometimes have `uncast` on and sometimes off? You may even have a quick comment about it at the use-site.

I improved the comment in the new commit.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23617#discussion_r2018234487
PR Review Comment: https://git.openjdk.org/jdk/pull/23617#discussion_r2018236157