RFR: 8345299: C2: some nodes can still have incorrect control after do_range_check()

[Vladimir Kozlov]

On Mon, 2 Dec 2024 13:53:00 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> 8339733 fixed controls for updated pre/main limits during
> do_range_check(). However, it missed one issue:
> 
> Control for the new limits is computed in `new_limit_ctrl` for both
> pre and main loops. `new_limit_ctrl` is currently initialized from the
> pre limit control but it also needs to take the main loop limit
> control into account as sometimes, the main loop limit control is
> below the pre limit and pre loop entry control.
> 
> 8339733 also introduced a couple bugs. Control of the `Bool`/`Cmp`
> nodes are updated for the pre and main loop to `new_limit_ctrl`. But
> that's incorrect because `new_limit_ctrl` may be above the pre loop
> while the `Bool`/`Cmp` for the pre loop are in the loop (because they
> depend on the loop iv) and for the main loop are after the pre loop
> (because they depend on the iv out of the pre loop). I fixed this for
> the pre loop by setting control for the `Bool`/`Cmp` to be as late as
> possible. For the main loop, no change appears to be required as
> control computed by c2 is already late enough. I've added an assert
> instead.

Few comments.

src/hotspot/share/opto/loopTransform.cpp line 2781:

> 2779:   // new pre_limit can push Bool/Cmp/Opaque nodes down (when one of the eliminated condition has parameters that are not
> 2780:   // loop invariant in the pre loop.
> 2781:   set_ctrl(pre_opaq, new_limit_ctrl);

Can you update this comment to explain different control settings as you did in PR's description.

src/hotspot/share/opto/loopTransform.cpp line 2825:

> 2823:   // new main_limit can push Bool/Cmp nodes down (when one of the eliminated condition has parameters that are not loop
> 2824:   // invariant in the pre loop).
> 2825:   set_ctrl(opqzm, new_limit_ctrl);

Update this comment too.

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

PR Review: https://git.openjdk.org/jdk/pull/22485#pullrequestreview-2476310277
PR Review Comment: https://git.openjdk.org/jdk/pull/22485#discussion_r1868102387
PR Review Comment: https://git.openjdk.org/jdk/pull/22485#discussion_r1868111672