RFR: 8339733: C2: some nodes can have incorrect control after do_range_check() [v3]

[Christian Hagedorn]

On Wed, 11 Sep 2024 08:10:45 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> PhaseIdealLoop::do_range_check() sets the control of the new pre and
>> main limits to be the entry control of the pre loop but it eliminates
>> all conditions whose parameters are invariant in the main loop. Most
>> of the time they are also invariant in the pre loop but that's not
>> guaranteed. It does happen sometimes that those parameters are pinned
>> in the pre loop. In that case, PhaseIdealLoop::do_range_check() sets
>> wrong controls.
>> 
>> This doesn't cause any issue today AFAICT.
>> 
>> Also, this seems to be a typo in PhaseIdealLoop::insert_pre_post_loops():
>> 
>> 
>> pre_head->in(0)
>> 
>> 
>> is `pre_head`. I fixed that one too.
>
> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review

Thanks for the update, looks good!

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

> 2892:       int scale_con= 1;        // Assume trip counter not scaled
> 2893: 
> 2894:       Node *limit_ctrl = get_ctrl(limit);

While at it:
Suggestion:

      Node* limit_ctrl = get_ctrl(limit);

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

> 2921:       }
> 2922: 
> 2923:       Node *offset_ctrl = get_ctrl(offset);

Suggestion:

      Node* offset_ctrl = get_ctrl(offset);

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20908#pullrequestreview-2296188147
PR Review Comment: https://git.openjdk.org/jdk/pull/20908#discussion_r1753768884
PR Review Comment: https://git.openjdk.org/jdk/pull/20908#discussion_r1753770237