RFR: 8353290: C2: Refactor PhaseIdealLoop::is_counted_loop() [v29]

[Christian Hagedorn]

On Wed, 14 Jan 2026 16:38:25 GMT, Kangcheng Xu <kxu at openjdk.org> wrote:

>> This PR refactors `PhaseIdealLoop::is_counted_loop()` into (mostly) `CountedLoopConverter::is_counted_loop()` and `CountedLoopConverter::convert()` to decouple the detection and conversion code. This enables us to try different loop configurations easily and finally convert once a counted loop is found. 
>> 
>> A nested `PhaseIdealLoop::CountedLoopConverter` class is created to handle the context, but I'm not if this is the best name or place for it. Please let me know what you think.
>> 
>> Blocks [JDK-8336759](https://bugs.openjdk.org/browse/JDK-8336759).
>
> Kangcheng Xu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix safepoint detection

I had a look at the failure and it seems that we miss a Loop Limit Check Predicate when we stress with `StressLongCountedLoop`. With the attached `Test.java` at the bottom, the following happens:

#### Mainline before your patch

We try to convert to a counted loop. We pass most of the checks, then emit a Loop Limit Check Predicate for "i < z" but then we bail out when `StressLongCountedLoop` is set:
https://github.com/openjdk/jdk/blob/99b4e05d502b68844699faa025e0d5bd51135d8f/src/hotspot/share/opto/loopnode.cpp#L2424-L2430

When we try again next time, we find that we do not need the Loop Limit Check Predicate because `init_plus_stride_could_overflow` is false:
https://github.com/openjdk/jdk/blob/99b4e05d502b68844699faa025e0d5bd51135d8f/src/hotspot/share/opto/loopnode.cpp#L2325-L2327
`init_t->hi_as_long()` will be `MAX_INT` (coming from the `ConvI2L` which we created with `StressLongCountedLoop`) and `max_signed_integer(iv_bt)` is `MAX_LONG`. So, we conclude that no Loop Limit Check Predicate is needed - we already added it in the previous attempt before converting the int counted loop to a long counted loop.

#### With your patch

We changed now the logic to only emit the Loop Limit Check Predicate if we actually convert the loop to a counted loop. In the first iteration, we fail due to the delay with `StressLongCountedLoop`. We do not emit the Loop Limit Check Predicate. In the second try, we find, as in mainline, that no Loop Limit Check Predicate is required and we end up emitting no Loop Limit Check Predicates at all while we still did it in mainline.

We should fix that. I think you could try to extract the Loop Limit Check Predicate creation from `CountedLoopConverter::convert()` to a separate method and call it in case we bail out here:

  if (converter.is_counted_loop()) {
#ifdef ASSERT
    // Stress by converting int counted loops to long counted loops
    if (converter.should_stress_long_counted_loop() && converter.stress_long_counted_loop()) {
      return false;
    }
#endif


<details>
<summary>Test.java</summary>


// Run with:
// $ java -XX:StressLongCountedLoop=2000000 -XX:CompileOnly=Test::test* -Xcomp Test.java
public class Test {
    static int x, y, z;

    public static void main(String[] args) {
        test();
    }

    static void test() {
        int i = x; // Any int
        do {
            x += y; 
            i++; // Could overflow and thus we need a Loop Limit Check Predicate "i < z"
        } while (i < z);
    }
}


</details>

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

PR Comment: https://git.openjdk.org/jdk/pull/24458#issuecomment-3800170491