RFR: 8267265: Use new IR Test Framework to create tests for C2 Ideal transformations

Christian Hagedorn chagedorn at openjdk.java.net
Mon Feb 14 08:20:10 UTC 2022


On Thu, 10 Feb 2022 22:12:53 GMT, Cesar Soares <duke at openjdk.java.net> wrote:

> Hi, can I please get some reviews for this Pull Request? Here is a summary of the changes:
> 
> - Add tests, using the new IR-based test framework, for several of the Ideal transformations on Add, Sub, Mul, Div, Loop nodes and some simple Scalar Replacement transformations.
> - Add more default IR regex's to IR-based test framework.

Apart from some minor things and some possible missing cases when comparing the `int` vs `long` tests, it looks good to me!

test/hotspot/jtreg/compiler/c2/irTests/AddLNodeIdealizationTests.java line 73:

> 71:         Asserts.assertEQ((a - b) + (b + c), test6(a, b, c));
> 72:         Asserts.assertEQ((a - b) + (c + b), test7(a, b, c));
> 73:         Asserts.assertEQ((a - b) + (c - a), test8(a, b, c));

`AddI` additionally has 

Asserts.assertEQ((a - b) + (b - c), test8(a, b, c));

Was this left out on purpose?

test/hotspot/jtreg/compiler/c2/irTests/DivINodeIdealizationTests.java line 128:

> 126:     @IR(counts = {IRNode.MUL, "1",
> 127:                   IRNode.DIV, "1",
> 128:                   IRNode.TRAP, "1"

Maybe we could add a new default regex `IRNode.DIV_BY_ZERO_TRAP`?

test/hotspot/jtreg/compiler/c2/irTests/DivINodeIdealizationTests.java line 149:

> 147:                  })
> 148:     // Checks (x & -(2^c0)) / 2^c1 => (x >> c1) & (2^c0 >> c1) => (x >> c1) & c3 where 2^c0 > |2^c1| and c3 = 2^c0 >> c1
> 149:     // Having a large enough and in the dividend removes the need to account for rounding when converting to shifts and multiplies as in divByPow2()

Maybe you capitalize `and` -> `AND`, same further down. I first read it as a normal word "and" which was confusing :-)

test/hotspot/jtreg/compiler/c2/irTests/SubLNodeIdealizationTests.java line 79:

> 77:         Asserts.assertEQ(a*b - b*c        , test16(a, b, c));
> 78:         Asserts.assertEQ(a*c - b*c        , test17(a, b, c));
> 79:         Asserts.assertEQ(a*b - c*a        , test18(a, b, c));

Some cases are missing compared to `SubI`. Did you omit them on purpose?

test/hotspot/jtreg/compiler/c2/irTests/loopOpts/LoopIdealizationTests.java line 94:

> 92:                this.blackhole();
> 93:                i++;
> 94:            }

Wrong indentation.

test/hotspot/jtreg/compiler/c2/irTests/loopOpts/LoopIdealizationTests.java line 129:

> 127:         while (i < 500){
> 128:             this.blackhole();
> 129:             if (i == 0){

Spacing

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

PR: https://git.openjdk.java.net/jdk/pull/7434


More information about the hotspot-compiler-dev mailing list