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