RFR: 8336759: C2: int counted loop with long limit not recognized as counted loop [v2]

[Christian Hagedorn]

On Tue, 3 Dec 2024 21:48:59 GMT, Kangcheng Xu <kxu at openjdk.org> wrote:

>> This patch implements [JDK-8336759](https://bugs.openjdk.org/browse/JDK-8336759) that recognizes int counted loops with long limits.
>> 
>> Currently, patterns like `for ( int i =...; i < long_limit; ...)` where int `i` is implicitly promoted to long (i.e., `(long) i < long_limit`) is not recognized as (int) counted loop. This patch speculatively and optimistically converts long limits to ints and deoptimize if the limit is outside int range, allowing more optimization opportunities. 
>> 
>> In other words, it transforms 
>> 
>> 
>> for (int i = 0; (long) i < long_limit; i++) {...}
>> 
>> 
>> to 
>> 
>> 
>> if (int_min <= long_limit && long_limit <= int_max ) {
>>     for (int i = 0;  i < (int) long_limit; i++) {...}
>> } else {
>>     trap: loop_limit_check
>> }
>> 
>> 
>> This could benefit calls to APIs like `long MemorySegment#byteSize()` when iterating over a long limit.
>
> Kangcheng Xu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   implement suggested changes from @chhagedorn's review

Thanks for the update! Some minor last things, otherwise, it looks good. Let me give this a spin in our testing.

Note that tomorrow is the fork. If this is not a critical performance enhancement, you might want to wait until after the fork, so this only goes into JDK 25. This patch will enable more optimization opportunities due to being able to convert more loops to counted ones. This could have some side effects.

src/hotspot/share/opto/loopnode.cpp line 1618:

> 1616: 
> 1617: //------------------------------is_counted_loop--------------------------------
> 1618: bool PhaseIdealLoop::try_convert_to_counted_loop(Node* loop_head, IdealLoopTree*&loop, BasicType iv_bt) {

Suggestion:

bool PhaseIdealLoop::try_convert_to_counted_loop(Node* loop_head, IdealLoopTree*& loop, BasicType iv_bt) {

src/hotspot/share/opto/loopnode.cpp line 1639:

> 1637: //    trap: loop_limit_check
> 1638: //  }
> 1639: bool PhaseIdealLoop::convert_to_counted_loop_with_speculative_long_limit(Node* loop_head, IdealLoopTree*&loop,

Suggestion:

bool PhaseIdealLoop::convert_to_counted_loop_with_speculative_long_limit(Node* loop_head, IdealLoopTree*& loop,

src/hotspot/share/opto/loopnode.cpp line 1716:

> 1714: }
> 1715: 
> 1716: bool PhaseIdealLoop::convert_to_counted_loop(Node* loop_head, IdealLoopTree*&loop, BasicType iv_bt) {

Suggestion:

bool PhaseIdealLoop::convert_to_counted_loop(Node* loop_head, IdealLoopTree*& loop, BasicType iv_bt) {

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22449#pullrequestreview-2477421719
PR Review Comment: https://git.openjdk.org/jdk/pull/22449#discussion_r1868800358
PR Review Comment: https://git.openjdk.org/jdk/pull/22449#discussion_r1868801323
PR Review Comment: https://git.openjdk.org/jdk/pull/22449#discussion_r1868802820