RFR: 8342692: C2: long counted loop/long range checks: don't create loop-nest for short running loops [v28]

Fri May 23 07:03:00 UTC 2025

On Thu, 22 May 2025 14:08:20 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> To optimize a long counted loop and long range checks in a long or int
>> counted loop, the loop is turned into a loop nest. When the loop has
>> few iterations, the overhead of having an outer loop whose backedge is
>> never taken, has a measurable cost. Furthermore, creating the loop
>> nest usually causes one iteration of the loop to be peeled so
>> predicates can be set up. If the loop is short running, then it's an
>> extra iteration that's run with range checks (compared to an int
>> counted loop with int range checks).
>> 
>> This change doesn't create a loop nest when:
>> 
>> 1- it can be determined statically at loop nest creation time that the
>>    loop runs for a short enough number of iterations
>>   
>> 2- profiling reports that the loop runs for no more than ShortLoopIter
>>    iterations (1000 by default).
>>   
>> For 2-, a guard is added which is implemented as yet another predicate.
>> 
>> While this change is in principle simple, I ran into a few
>> implementation issues:
>> 
>> - while c2 has a way to compute the number of iterations of an int
>>   counted loop, it doesn't have that for long counted loop. The
>>   existing logic for int counted loops promotes values to long to
>>   avoid overflows. I reworked it so it now works for both long and int
>>   counted loops.
>> 
>> - I added a new deoptimization reason (Reason_short_running_loop) for
>>   the new predicate. Given the number of iterations is narrowed down
>>   by the predicate, the limit of the loop after transformation is a
>>   cast node that's control dependent on the short running loop
>>   predicate. Because once the counted loop is transformed, it is
>>   likely that range check predicates will be inserted and they will
>>   depend on the limit, the short running loop predicate has to be the
>>   one that's further away from the loop entry. Now it is also possible
>>   that the limit before transformation depends on a predicate
>>   (TestShortRunningLongCountedLoopPredicatesClone is an example), we
>>   can have: new predicates inserted after the transformation that
>>   depend on the casted limit that itself depend on old predicates
>>   added before the transformation. To solve this cicular dependency,
>>   parse and assert predicates are cloned between the old predicates
>>   and the loop head. The cloned short running loop parse predicate is
>>   the one that's used to insert the short running loop predicate.
>> 
>> - In the case of a long counted loop, the loop is transformed into a
>>   regular loop with a ...
>
> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review

A few minor last comments but otherwise, it looks good to me! Thanks for all the updates, the patience and the credit!

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

> 1233:     //  Template Assertion Predicates
> 1234:     //              |
> 1235:     //             Loop

Suggestion:

    //        Existing Hoisted
    //        Check Predicates
    //               |
    //     New Short Running Long
    //         Loop Predicate
    //               |
    //   Cloned Parse Predicates and
    //  Template Assertion Predicates
    //               |
    //             Loop

src/hotspot/share/opto/predicates.cpp line 86:

> 84: 
> 85: ParsePredicate ParsePredicate::clone_to_loop(Node* new_control, const bool rewire_uncommon_proj_phi_inputs,
> 86:                                                         PhaseIdealLoop* phase) const {

Suggestion:

ParsePredicate ParsePredicate::clone_to_loop(Node* new_control, const bool rewire_uncommon_proj_phi_inputs,
                                             PhaseIdealLoop* phase) const {

test/hotspot/jtreg/compiler/longcountedloops/TestShortRunningIntLoopWithLongChecksPredicates.java line 2:

> 1: /*
> 2:  * Copyright (c) 2024, Red Hat, Inc. All rights reserved.

Suggestion:

test/hotspot/jtreg/compiler/longcountedloops/TestShortRunningIntLoopWithLongChecksPredicates.java line 39:

> 37: // possible to optimize long RC. Finally unrolling happen which
> 38: // require the Assert Predicates to have been properly copied when the
> 39: // loop was transformed for the long range check.

Suggestion:

// require the Assertion Predicates to have been properly copied when
// the loop was transformed for the long range check.

test/hotspot/jtreg/compiler/longcountedloops/TestShortRunningLongCountedLoop.java line 2:

> 1: /*
> 2:  * Copyright (c) 2024, Red Hat, Inc. All rights reserved.

Suggestion:

test/hotspot/jtreg/compiler/longcountedloops/TestShortRunningLongCountedLoopPredicatesClone.java line 2:

> 1: /*
> 2:  * Copyright (c) 2024, Red Hat, Inc. All rights reserved.

Suggestion:

test/hotspot/jtreg/compiler/longcountedloops/TestShortRunningLongCountedLoopScaleOverflow.java line 2:

> 1: /*
> 2:  * Copyright (c) 2024, Red Hat, Inc. All rights reserved.

Suggestion:

test/micro/org/openjdk/bench/java/lang/foreign/HeapMismatchManualLoopTest.java line 2:

> 1: /*
> 2:  *  Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved.

Should probably also be 2025?
Suggestion:

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21630#pullrequestreview-2863375615
PR Review Comment: https://git.openjdk.org/jdk/pull/21630#discussion_r2103928708
PR Review Comment: https://git.openjdk.org/jdk/pull/21630#discussion_r2103920931
PR Review Comment: https://git.openjdk.org/jdk/pull/21630#discussion_r2103933089
PR Review Comment: https://git.openjdk.org/jdk/pull/21630#discussion_r2103923180
PR Review Comment: https://git.openjdk.org/jdk/pull/21630#discussion_r2103933286
PR Review Comment: https://git.openjdk.org/jdk/pull/21630#discussion_r2103933791
PR Review Comment: https://git.openjdk.org/jdk/pull/21630#discussion_r2103933982
PR Review Comment: https://git.openjdk.org/jdk/pull/21630#discussion_r2103934808