RFR: 8341407: C2: assert(main_limit == cl->limit() || get_ctrl(main_limit) == new_limit_ctrl) failed: wrong control for added limit [v2]

[Tobias Hartmann]

On Thu, 17 Oct 2024 14:54:51 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> That assert checks that during RC elimination, we have either:
>> 
>> - not updated the limit of the main loop
>> 
>> - or that the new limit is at the expected control
>> 
>> The assert fires because the limit was updated but is not at the
>> expected control. That happens because `new_limit_ctrl` is updated for
>> a test that it attempts to eliminate before it actually proceeds with
>> the elimination: if the test can't be eliminated, `new_limit_ctrl`
>> gets updated anyway.
>> 
>> While the assert could, maybe, be relaxed (it fires in this case but
>> nothing is going wrong), it's better, I think, to simply not uselessly
>> restrict the control of the limit.
>
> Roland Westrelin has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit:
> 
>   fix & test

Looks good to me.

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

> 3002:         }
> 3003:       }
> 3004:       // Only updated variable tracking control for new nodes if it's indeed a range check that can be eliminated (and

Suggestion:

      // Only update variable tracking control for new nodes if it's indeed a range check that can be eliminated (and

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

Marked as reviewed by thartmann (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21564#pullrequestreview-2377224737
PR Review Comment: https://git.openjdk.org/jdk/pull/21564#discussion_r1806006352