While investigating JDK-8373134, Roland gave me a hand creating the following test case that failed with VerifyIterativeGVN:
```
public class TestIntMax
{
private static int test(int a, int b)
{
int i;
for (i = -10; i < 1; i++)
{
}
int c = a * i;
return Integer.max(a, Integer.max(b, c));
}
public static void main(String[] args)
{
for (int i = 0; i < 20_000; i++)
{
test(42, 42);
}
}
}
```
To understand why it didn't work, I compared to what was happening with the same case if applied to long and that one worked. So, it seemed like the case above was not really caused byJDK-8373134 but by something else.
After digging I realised that long worked because MaxLNode::Ideal calls AddNode::Ideal which in turn calls commute and that causes an additional optimization that makes MaxLNode work. When compared with MaxINode, there's no calls to AddNode::Ideal, so there is a bunch of optimisations missing for MaxI nodes.
The following IR test, inspired by a simple Roland test case created, highlights the commutation issue:
```
package compiler.c2.templated;
/mport compiler.lib.generators.*;
import compiler.lib.ir_framework.*;
import compiler.lib.verify.*;
public class MinMaxIdeal {
public static void main(String[] vmFlags) {
TestFramework framework = new TestFramework(MinMaxIdeal.class);
framework.addFlags(
"-classpath",
"/Users/g/src/jdk-commute-et-al/JTwork/classes/compiler/c2/irTests/TestMinMaxIdeal.d:/Users/g/src/jdk-commute-et-al/test/hotspot/jtreg/compiler/c2/irTests:/Users/g/src/jdk-commute-et-al/JTwork/classes/compiler/c2/irTests/TestMinMaxIdeal.d/test/lib:/Users/g/src/jdk-commute-et-al/test/lib:/Users/g/src/jdk-commute-et-al/test/hotspot/jtreg:/Users/g/opt/jtreg/build/images/jtreg/lib/javatest.jar:/Users/g/opt/jtreg/build/images/jtreg/lib/jtreg.jar:/Users/g/src/jdk-commute-et-al/JTwork/scratch/./compile-framework-classes-15731717948002588134");
framework.addFlags(vmFlags);
framework.start();
}
@Test
@IR(counts = {IRNode.MAX_I, "= 1"})
@Arguments(values = {Argument.NUMBER_42, Argument.NUMBER_42})
public int test_MAX_I(int a, int b) {
return Integer.max(a, b) + Integer.max(b, a);
}
}
```
Fails with:
```
Failed IR Rules (1) of Methods (1)
----------------------------------
1) Method "public int compiler.c2.templated.MinMaxIdeal.test_MAX_I(int,int)" - [Failed IR rules: 1]:
* @IR rule 1: "@compiler.lib.ir_framework.IR(phase={DEFAULT}, applyIfPlatformAnd={}, applyIfCPUFeatureOr={}, counts={"_#MAX_I#_", "= 1"}, failOn={}, applyIfPlatform={}, applyIfPlatformOr={}, applyIfOr={}, applyIfCPUFeatureAnd={}, applyIf={}, applyIfCPUFeature={}, applyIfAnd={}, applyIfNot={})"
> Phase "PrintIdeal":
- counts: Graph contains wrong number of nodes:
* Constraint 1: "(\d+(\s){2}(MaxI.*)+(\s){2}===.*)"
- Failed comparison: [found] 2 = 1 [given]
- Matched nodes (2):
* 34 MaxI === _ 11 12 [[ 47 ]] !jvms: Integer::max @ bci:2 (line 1934) MinMaxIdeal::test_MAX_I @ bci:2 (line 23)
* 46 MaxI === _ 12 11 [[ 47 ]] !jvms: Integer::max @ bci:2 (line 1934) MinMaxIdeal::test_MAX_I @ bci:7 (line 23)
>>> Check stdout for compilation output of the failed methods
```
The IR test has been generated with the template framework which is attached.
The aim of this this bug is to add AddNode::Ideal to MaxINode::Ideal and create tests that verify that commutation and other optimisations inside AddNode::Ideal apply correctly. The test will also explore what happens with other min/max node types (float, half float, double, long) but if any issues appear there, a separate bug will be filled.
```
public class TestIntMax
{
private static int test(int a, int b)
{
int i;
for (i = -10; i < 1; i++)
{
}
int c = a * i;
return Integer.max(a, Integer.max(b, c));
}
public static void main(String[] args)
{
for (int i = 0; i < 20_000; i++)
{
test(42, 42);
}
}
}
```
To understand why it didn't work, I compared to what was happening with the same case if applied to long and that one worked. So, it seemed like the case above was not really caused by
After digging I realised that long worked because MaxLNode::Ideal calls AddNode::Ideal which in turn calls commute and that causes an additional optimization that makes MaxLNode work. When compared with MaxINode, there's no calls to AddNode::Ideal, so there is a bunch of optimisations missing for MaxI nodes.
The following IR test, inspired by a simple Roland test case created, highlights the commutation issue:
```
package compiler.c2.templated;
/mport compiler.lib.generators.*;
import compiler.lib.ir_framework.*;
import compiler.lib.verify.*;
public class MinMaxIdeal {
public static void main(String[] vmFlags) {
TestFramework framework = new TestFramework(MinMaxIdeal.class);
framework.addFlags(
"-classpath",
"/Users/g/src/jdk-commute-et-al/JTwork/classes/compiler/c2/irTests/TestMinMaxIdeal.d:/Users/g/src/jdk-commute-et-al/test/hotspot/jtreg/compiler/c2/irTests:/Users/g/src/jdk-commute-et-al/JTwork/classes/compiler/c2/irTests/TestMinMaxIdeal.d/test/lib:/Users/g/src/jdk-commute-et-al/test/lib:/Users/g/src/jdk-commute-et-al/test/hotspot/jtreg:/Users/g/opt/jtreg/build/images/jtreg/lib/javatest.jar:/Users/g/opt/jtreg/build/images/jtreg/lib/jtreg.jar:/Users/g/src/jdk-commute-et-al/JTwork/scratch/./compile-framework-classes-15731717948002588134");
framework.addFlags(vmFlags);
framework.start();
}
@Test
@IR(counts = {IRNode.MAX_I, "= 1"})
@Arguments(values = {Argument.NUMBER_42, Argument.NUMBER_42})
public int test_MAX_I(int a, int b) {
return Integer.max(a, b) + Integer.max(b, a);
}
}
```
Fails with:
```
Failed IR Rules (1) of Methods (1)
----------------------------------
1) Method "public int compiler.c2.templated.MinMaxIdeal.test_MAX_I(int,int)" - [Failed IR rules: 1]:
* @IR rule 1: "@compiler.lib.ir_framework.IR(phase={DEFAULT}, applyIfPlatformAnd={}, applyIfCPUFeatureOr={}, counts={"_#MAX_I#_", "= 1"}, failOn={}, applyIfPlatform={}, applyIfPlatformOr={}, applyIfOr={}, applyIfCPUFeatureAnd={}, applyIf={}, applyIfCPUFeature={}, applyIfAnd={}, applyIfNot={})"
> Phase "PrintIdeal":
- counts: Graph contains wrong number of nodes:
* Constraint 1: "(\d+(\s){2}(MaxI.*)+(\s){2}===.*)"
- Failed comparison: [found] 2 = 1 [given]
- Matched nodes (2):
* 34 MaxI === _ 11 12 [[ 47 ]] !jvms: Integer::max @ bci:2 (line 1934) MinMaxIdeal::test_MAX_I @ bci:2 (line 23)
* 46 MaxI === _ 12 11 [[ 47 ]] !jvms: Integer::max @ bci:2 (line 1934) MinMaxIdeal::test_MAX_I @ bci:7 (line 23)
>>> Check stdout for compilation output of the failed methods
```
The IR test has been generated with the template framework which is attached.
The aim of this this bug is to add AddNode::Ideal to MaxINode::Ideal and create tests that verify that commutation and other optimisations inside AddNode::Ideal apply correctly. The test will also explore what happens with other min/max node types (float, half float, double, long) but if any issues appear there, a separate bug will be filled.
- relates to
-
JDK-8373134 C2: Min/Max users of Min/Max uses should be enqueued for GVN
-
- Resolved
-
- links to
-
Commit(master)
openjdk/jdk/2c0d9a79
-
Review(master)
openjdk/jdk/28770