RFR: 8328528: C2 should optimize long-typed parallel iv in an int counted loop [v19]

Tobias Hartmann thartmann at openjdk.org
Wed Oct 2 11:29:40 UTC 2024


On Mon, 30 Sep 2024 13:36:19 GMT, Kangcheng Xu <kxu at openjdk.org> wrote:

>> Currently, parallel iv optimization only happens in an int counted loop with int-typed parallel iv's. This PR adds support for long-typed iv to be optimized. 
>> 
>> Additionally, this ticket contributes to the resolution of [JDK-8275913](https://bugs.openjdk.org/browse/JDK-8275913). Meanwhile, I'm working on adding support for parallel IV replacement for long counted loops which will depend on this PR.
>
> Kangcheng Xu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   update comments in TestParallelIvInIntCountedLoop.java

Changes requested by thartmann (Reviewer).

test/hotspot/jtreg/compiler/loopopts/parallel_iv/TestParallelIvInIntCountedLoop.java line 315:

> 313:         }
> 314: 
> 315:         return a;

Shouldn't there also be tests for the `int a` `long i` variant?

test/hotspot/jtreg/compiler/loopopts/parallel_iv/TestParallelIvInIntCountedLoop.java line 319:

> 317: 
> 318:     private static void testCorrectness() {
> 319:         Random rng = new Random();

You should use `Utils.getRandomInstance()` instead which logs the seed for better reproducibility. Also add `@key randomness` to the test header.

test/hotspot/jtreg/compiler/loopopts/parallel_iv/TestParallelIvInIntCountedLoop.java line 321:

> 319:         Random rng = new Random();
> 320: 
> 321:         // Since we can't easily determined expected values if loop varibles overflow, we make sure i is less than (MAX_VALUE - stride).

Suggestion:

        // Since we can't easily determine expected values if loop variables overflow, we make sure i is less than (MAX_VALUE - stride).

test/hotspot/jtreg/compiler/loopopts/parallel_iv/TestParallelIvInIntCountedLoop.java line 325:

> 323: 
> 324:         for (int i : iterations) {
> 325:             Asserts.assertEQ(i, testIntCountedLoopWithIntIV(i));

Code in this loop is not guaranteed to be even C2 compiled because IR verification will be executed in a separate VM. IR framework tests that also want to verify the output, should be written like this:

https://github.com/openjdk/jdk/blob/9bd478593cc92a716151d1373f3426f1d92143bb/test/hotspot/jtreg/testlibrary_tests/ir_framework/examples/CustomRunTestExample.java#L84-L97

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

PR Review: https://git.openjdk.org/jdk/pull/18489#pullrequestreview-2342647468
PR Review Comment: https://git.openjdk.org/jdk/pull/18489#discussion_r1784324768
PR Review Comment: https://git.openjdk.org/jdk/pull/18489#discussion_r1784331782
PR Review Comment: https://git.openjdk.org/jdk/pull/18489#discussion_r1784325142
PR Review Comment: https://git.openjdk.org/jdk/pull/18489#discussion_r1784333260


More information about the hotspot-compiler-dev mailing list