Skip to content

Commit f37acef

Browse files
gergo-gergo-
committed
[GR-68934] Canonicalize conditionals to min/max nodes
PullRequest: graal/21964
2 parents 5d53f89 + e7b3a67 commit f37acef

File tree

29 files changed

+633
-96
lines changed

29 files changed

+633
-96
lines changed

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/MinMaxCanonicalizationTest.java‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/*
2+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation. Oracle designates this
8+
* particular file as subject to the "Classpath" exception as provided
9+
* by Oracle in the LICENSE file that accompanied this code.
10+
*
11+
* This code is distributed in the hope that it will be useful, but WITHOUT
12+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
13+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
14+
* version 2 for more details (a copy is included in the LICENSE file that
15+
* accompanied this code).
16+
*
17+
* You should have received a copy of the GNU General Public License version
18+
* 2 along with this work; if not, write to the Free Software Foundation,
19+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
20+
*
21+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
22+
* or visit www.oracle.com if you need additional information or have any
23+
* questions.
24+
*/
25+
26+
package jdk.graal.compiler.core.test;
27+
28+
import org.junit.Test;
29+
30+
import jdk.graal.compiler.core.common.NumUtil;
31+
import jdk.graal.compiler.nodes.StructuredGraph;
32+
import jdk.vm.ci.meta.ResolvedJavaMethod;
33+
34+
public class MinMaxCanonicalizationTest extends GraalCompilerTest {
35+
36+
record ByteHolder(byte b) {
37+
38+
}
39+
40+
public static boolean byteMinSnippet(ByteHolder holder) {
41+
byte b = holder.b;
42+
/* This tests a case in IntegerEqualsOp#canonical where we need constants in the graph. */
43+
return (b < 8 ? b : 8) == 7;
44+
}
45+
46+
public static boolean byteMinReference(ByteHolder holder) {
47+
return holder.b == 7;
48+
}
49+
50+
@Test
51+
public void testByteMin() {
52+
testByteHolderMethod("byteMinSnippet", "byteMinReference");
53+
}
54+
55+
private void testByteHolderMethod(String testSnippetName, String referenceSnippetName) {
56+
StructuredGraph graph = parseEager(testSnippetName, StructuredGraph.AllowAssumptions.YES);
57+
createInliningPhase().apply(graph, getDefaultHighTierContext());
58+
createCanonicalizerPhase().apply(graph, getProviders());
59+
60+
StructuredGraph referenceGraph = parseEager(referenceSnippetName, StructuredGraph.AllowAssumptions.YES);
61+
assertEquals(referenceGraph, graph);
62+
63+
ResolvedJavaMethod referenceMethod = referenceGraph.method();
64+
for (byte b = 7; b <= 9; b++) {
65+
testAgainstExpected(graph.method(), executeExpected(referenceMethod, null, new ByteHolder(b)), null, new ByteHolder(b));
66+
}
67+
}
68+
69+
public static boolean byteMaxEqualsToLessThanSnippet(ByteHolder holder) {
70+
return Math.max(7, holder.b) == 7;
71+
}
72+
73+
public static boolean byteMaxEqualsToLessThanReference(ByteHolder holder) {
74+
return holder.b <= 7;
75+
}
76+
77+
@Test
78+
public void testByteMaxEqualsToLessThan() {
79+
testByteHolderMethod("byteMaxEqualsToLessThanSnippet", "byteMaxEqualsToLessThanReference");
80+
}
81+
82+
private static volatile byte symbolic = 7;
83+
84+
public static boolean byteMaxEqualsToLessThanSymbolicSnippet(ByteHolder holder) {
85+
byte other = symbolic;
86+
/*
87+
* Many of these tests test a case where we don't need constants in the graph. The tests
88+
* look cleaner with constants, but here is a demonstration that fully symbolic reasoning
89+
* works too.
90+
*/
91+
return Math.max(other, holder.b) == other;
92+
}
93+
94+
public static boolean byteMaxEqualsToLessThanSymbolicReference(ByteHolder holder) {
95+
byte other = symbolic;
96+
return holder.b <= other;
97+
}
98+
99+
@Test
100+
public void testByteMaxEqualsToLessThanSymbolic() {
101+
testByteHolderMethod("byteMaxEqualsToLessThanSymbolicSnippet", "byteMaxEqualsToLessThanSymbolicReference");
102+
}
103+
104+
public static boolean byteUMaxEqualsToLessThanSnippet(ByteHolder holder) {
105+
return NumUtil.maxUnsigned(holder.b & 0xff, 7) == 7;
106+
}
107+
108+
public static boolean byteUMaxEqualsToLessThanReference(ByteHolder holder) {
109+
return Byte.compareUnsigned(holder.b, (byte) 7) <= 0;
110+
}
111+
112+
@Test
113+
public void testByteUMaxEqualsToLessThan() {
114+
testByteHolderMethod("byteUMaxEqualsToLessThanSnippet", "byteUMaxEqualsToLessThanReference");
115+
}
116+
117+
public static boolean byteMinEqualsToLessThanSnippet(ByteHolder holder) {
118+
return Math.min(7, holder.b) == 7;
119+
}
120+
121+
public static boolean byteMinEqualsToLessThanReference(ByteHolder holder) {
122+
return holder.b >= 7;
123+
}
124+
125+
@Test
126+
public void testByteMinEqualsToLessThan() {
127+
testByteHolderMethod("byteMinEqualsToLessThanSnippet", "byteMinEqualsToLessThanReference");
128+
}
129+
130+
public static boolean byteUMinEqualsToLessThanSnippet(ByteHolder holder) {
131+
return NumUtil.minUnsigned(holder.b & 0xff, 7) == 7;
132+
}
133+
134+
public static boolean byteUMinEqualsToLessThanReference(ByteHolder holder) {
135+
return Byte.compareUnsigned(holder.b, (byte) 7) >= 0;
136+
}
137+
138+
@Test
139+
public void testByteUMinEqualsToLessThan() {
140+
testByteHolderMethod("byteUMinEqualsToLessThanSnippet", "byteUMinEqualsToLessThanReference");
141+
}
142+
}

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/nodes/test/IfNodeCanonicalizationTest2.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved.
33
* Copyright (c) 2019, Arm Limited. All rights reserved.
44
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
55
*
@@ -26,14 +26,16 @@
2626

2727
package jdk.graal.compiler.nodes.test;
2828

29+
import org.junit.Assert;
30+
import org.junit.Test;
31+
2932
import jdk.graal.compiler.core.test.GraalCompilerTest;
3033
import jdk.graal.compiler.core.test.TestPhase;
3134
import jdk.graal.compiler.nodes.StructuredGraph;
3235
import jdk.graal.compiler.nodes.calc.ConditionalNode;
36+
import jdk.graal.compiler.nodes.calc.MinMaxNode;
3337
import jdk.graal.compiler.options.OptionValues;
3438
import jdk.graal.compiler.phases.tiers.Suites;
35-
import org.junit.Assert;
36-
import org.junit.Test;
3739

3840
public class IfNodeCanonicalizationTest2 extends GraalCompilerTest {
3941
private StructuredGraph structuredGraph;
@@ -162,7 +164,7 @@ public void testIsNullCondMove() {
162164

163165
private void checkConditionalNode(String methodName, int expected) {
164166
compile(getResolvedJavaMethod(methodName), null);
165-
int actual = structuredGraph.getNodes().filter(ConditionalNode.class).count();
167+
int actual = structuredGraph.getNodes().filter(n -> n instanceof ConditionalNode || n instanceof MinMaxNode<?>).count();
166168
Assert.assertEquals(expected, actual);
167169
}
168170
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/aarch64/AArch64ArithmeticLIRGenerator.java‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import jdk.graal.compiler.asm.aarch64.AArch64MacroAssembler;
3636
import jdk.graal.compiler.core.common.LIRKind;
3737
import jdk.graal.compiler.core.common.NumUtil;
38+
import jdk.graal.compiler.core.common.calc.Condition;
3839
import jdk.graal.compiler.core.common.calc.FloatConvert;
3940
import jdk.graal.compiler.core.common.memory.MemoryExtendKind;
4041
import jdk.graal.compiler.core.common.memory.MemoryOrderMode;
@@ -469,21 +470,48 @@ public Value emitMathAbs(Value input) {
469470
}
470471

471472
@Override
472-
public Value emitMathMax(Value a, Value b) {
473+
public Value emitMathMax(LIRKind cmpKind, Value a, Value b) {
474+
if (((AArch64Kind) cmpKind.getPlatformKind()).isInteger()) {
475+
return emitIntegerMinMax(cmpKind, a, b, AArch64ArithmeticOp.SMAX);
476+
}
473477
assert a.getPlatformKind() == b.getPlatformKind() : "Kind must match a=" + a + " b=" + b;
474478
assert a.getPlatformKind() == AArch64Kind.DOUBLE ||
475479
a.getPlatformKind() == AArch64Kind.SINGLE : a;
476480
return emitBinary(LIRKind.combine(a, b), AArch64ArithmeticOp.FMAX, true, a, b);
477481
}
478482

479483
@Override
480-
public Value emitMathMin(Value a, Value b) {
484+
public Value emitMathMin(LIRKind cmpKind, Value a, Value b) {
485+
if (((AArch64Kind) cmpKind.getPlatformKind()).isInteger()) {
486+
return emitIntegerMinMax(cmpKind, a, b, AArch64ArithmeticOp.SMIN);
487+
}
481488
assert a.getPlatformKind() == b.getPlatformKind() : "Kind must match a=" + a + " b=" + b;
482489
assert a.getPlatformKind() == AArch64Kind.DOUBLE ||
483490
a.getPlatformKind() == AArch64Kind.SINGLE : a;
484491
return emitBinary(LIRKind.combine(a, b), AArch64ArithmeticOp.FMIN, true, a, b);
485492
}
486493

494+
@Override
495+
public Value emitMathUnsignedMax(LIRKind cmpKind, Value a, Value b) {
496+
return emitIntegerMinMax(cmpKind, a, b, AArch64ArithmeticOp.UMAX);
497+
}
498+
499+
@Override
500+
public Value emitMathUnsignedMin(LIRKind cmpKind, Value a, Value b) {
501+
return emitIntegerMinMax(cmpKind, a, b, AArch64ArithmeticOp.UMIN);
502+
}
503+
504+
private Value emitIntegerMinMax(LIRKind cmpKind, Value a, Value b, AArch64ArithmeticOp minMaxOp) {
505+
Condition condition = switch (minMaxOp) {
506+
case SMAX -> Condition.GE;
507+
case SMIN -> Condition.LE;
508+
case UMAX -> Condition.AE;
509+
case UMIN -> Condition.BE;
510+
default -> throw GraalError.shouldNotReachHereUnexpectedValue(minMaxOp);
511+
};
512+
return getLIRGen().emitConditionalMove(cmpKind.getPlatformKind(), a, b, condition, false, a, b);
513+
}
514+
487515
@Override
488516
public Value emitMathSqrt(Value input) {
489517
assert input.getPlatformKind() == AArch64Kind.DOUBLE ||

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/amd64/AMD64ArithmeticLIRGenerator.java‎

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@
112112
import jdk.graal.compiler.asm.amd64.AVXKind.AVXSize;
113113
import jdk.graal.compiler.core.common.LIRKind;
114114
import jdk.graal.compiler.core.common.NumUtil;
115+
import jdk.graal.compiler.core.common.calc.Condition;
115116
import jdk.graal.compiler.core.common.calc.FloatConvert;
116117
import jdk.graal.compiler.core.common.calc.FloatConvertCategory;
117118
import jdk.graal.compiler.core.common.memory.MemoryExtendKind;
@@ -201,6 +202,26 @@ protected void setLirGen(LIRGenerator lirGen) {
201202
private final AllocatableValue nullRegisterValue;
202203
protected AMD64SIMDInstructionEncoding simdEncoding;
203204

205+
protected Value emitIntegerMinMax(LIRKind cmpKind, Value a, Value b, AMD64MathMinMaxFloatOp minMaxOp, NumUtil.Signedness signedness) {
206+
AMD64Kind kind = (AMD64Kind) a.getPlatformKind();
207+
GraalError.guarantee(kind.isInteger(), "unexpected value in superclass: %s", a);
208+
Condition condition;
209+
if (minMaxOp == AMD64MathMinMaxFloatOp.Min) {
210+
if (signedness == NumUtil.Signedness.SIGNED) {
211+
condition = Condition.LE;
212+
} else {
213+
condition = Condition.BE;
214+
}
215+
} else {
216+
if (signedness == NumUtil.Signedness.SIGNED) {
217+
condition = Condition.GE;
218+
} else {
219+
condition = Condition.AE;
220+
}
221+
}
222+
return getLIRGen().emitConditionalMove(cmpKind.getPlatformKind(), a, b, condition, false, a, b);
223+
}
224+
204225
@Override
205226
public Variable emitNegate(Value inputVal, boolean setFlags) {
206227
AllocatableValue input = asAllocatable(inputVal);
@@ -1672,13 +1693,29 @@ public Variable emitVectorBlend(Value zeroValue, Value oneValue, Value mask) {
16721693
}
16731694

16741695
@Override
1675-
public Value emitMathMax(Value x, Value y) {
1676-
return emitMathMinMax(x, y, AMD64MathMinMaxFloatOp.Max);
1696+
public Value emitMathMax(LIRKind cmpKind, Value x, Value y) {
1697+
if (((AMD64Kind) cmpKind.getPlatformKind()).isInteger()) {
1698+
return emitIntegerMinMax(cmpKind, x, y, AMD64MathMinMaxFloatOp.Max, NumUtil.Signedness.SIGNED);
1699+
}
1700+
return emitMathMinMax(cmpKind, x, y, AMD64MathMinMaxFloatOp.Max);
1701+
}
1702+
1703+
@Override
1704+
public Value emitMathMin(LIRKind cmpKind, Value x, Value y) {
1705+
if (((AMD64Kind) cmpKind.getPlatformKind()).isInteger()) {
1706+
return emitIntegerMinMax(cmpKind, x, y, AMD64MathMinMaxFloatOp.Min, NumUtil.Signedness.SIGNED);
1707+
}
1708+
return emitMathMinMax(cmpKind, x, y, AMD64MathMinMaxFloatOp.Min);
1709+
}
1710+
1711+
@Override
1712+
public Value emitMathUnsignedMax(LIRKind cmpKind, Value x, Value y) {
1713+
return emitIntegerMinMax(cmpKind, x, y, AMD64MathMinMaxFloatOp.Max, NumUtil.Signedness.UNSIGNED);
16771714
}
16781715

16791716
@Override
1680-
public Value emitMathMin(Value x, Value y) {
1681-
return emitMathMinMax(x, y, AMD64MathMinMaxFloatOp.Min);
1717+
public Value emitMathUnsignedMin(LIRKind cmpKind, Value x, Value y) {
1718+
return emitIntegerMinMax(cmpKind, x, y, AMD64MathMinMaxFloatOp.Min, NumUtil.Signedness.UNSIGNED);
16821719
}
16831720

16841721
protected enum AMD64MathMinMaxFloatOp {
@@ -1740,7 +1777,8 @@ public boolean isPreferredZeroConst(JavaConstant constant) {
17401777
* @see Math#min(double, double)
17411778
* @see Math#min(float, float)
17421779
*/
1743-
protected Value emitMathMinMax(Value a, Value b, AMD64MathMinMaxFloatOp minmaxop) {
1780+
@SuppressWarnings("unused")
1781+
protected Value emitMathMinMax(LIRKind cmpKind, Value a, Value b, AMD64MathMinMaxFloatOp minmaxop) {
17441782
assert supportAVX();
17451783
AMD64Kind kind = (AMD64Kind) a.getPlatformKind();
17461784

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/lir/gen/ArithmeticLIRGeneratorTool.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,22 +185,22 @@ default Value emitMathPow(Value x, Value y) {
185185
}
186186

187187
@SuppressWarnings("unused")
188-
default Value emitMathMax(Value x, Value y) {
188+
default Value emitMathMax(LIRKind cmpKind, Value x, Value y) {
189189
throw GraalError.unimplemented("No specialized implementation available"); // ExcludeFromJacocoGeneratedReport
190190
}
191191

192192
@SuppressWarnings("unused")
193-
default Value emitMathMin(Value x, Value y) {
193+
default Value emitMathMin(LIRKind cmpKind, Value x, Value y) {
194194
throw GraalError.unimplemented("No specialized implementation available"); // ExcludeFromJacocoGeneratedReport
195195
}
196196

197197
@SuppressWarnings("unused")
198-
default Value emitMathUnsignedMax(Value x, Value y) {
198+
default Value emitMathUnsignedMax(LIRKind cmpKind, Value x, Value y) {
199199
throw GraalError.unimplemented("No specialized implementation available"); // ExcludeFromJacocoGeneratedReport
200200
}
201201

202202
@SuppressWarnings("unused")
203-
default Value emitMathUnsignedMin(Value x, Value y) {
203+
default Value emitMathUnsignedMin(LIRKind cmpKind, Value x, Value y) {
204204
throw GraalError.unimplemented("No specialized implementation available"); // ExcludeFromJacocoGeneratedReport
205205
}
206206

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/calc/BinaryArithmeticNode.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2009, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2009, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -603,11 +603,17 @@ public static ValueNode reassociateUnmatchedValues(BinaryArithmeticNode<?> node,
603603
// Re-association from "x ^ (y ^ C)" to "(x ^ y) ^ C"
604604
return XorNode.create(matchValue, XorNode.create(otherValue1, otherValue2, view), view);
605605
} else if (node instanceof MinNode) {
606-
// Re-association from "Math.min(x, Math.min(y, C))" to "Math.min(Math.min(x, y), C)"
606+
// Re-association from "min(x, min(y, C))" to "min(min(x, y), C)"
607607
return MinNode.create(matchValue, MinNode.create(otherValue1, otherValue2, view), view);
608608
} else if (node instanceof MaxNode) {
609-
// Re-association from "Math.max(x, Math.max(y, C))" to "Math.max(Math.max(x, y), C)"
609+
// Re-association from "max(x, max(y, C))" to "max(max(x, y), C)"
610610
return MaxNode.create(matchValue, MaxNode.create(otherValue1, otherValue2, view), view);
611+
} else if (node instanceof UnsignedMinNode) {
612+
// Re-association from "umin(x, umin(y, C))" to "umin(umin(x, y), C)"
613+
return UnsignedMinNode.create(matchValue, UnsignedMinNode.create(otherValue1, otherValue2, view), view);
614+
} else if (node instanceof UnsignedMaxNode) {
615+
// Re-association from "umax(x, umax(y, C))" to "umax(umax(x, y), C)"
616+
return UnsignedMaxNode.create(matchValue, UnsignedMaxNode.create(otherValue1, otherValue2, view), view);
611617
} else {
612618
throw GraalError.shouldNotReachHere("unhandled node in reassociation with constants: " + node); // ExcludeFromJacocoGeneratedReport
613619
}
@@ -721,6 +727,10 @@ public static ValueNode reassociateMatchedValues(BinaryArithmeticNode<?> node, N
721727
return MaxNode.create(a, MaxNode.create(m1, m2, view), view);
722728
} else if (node instanceof MinNode) {
723729
return MinNode.create(a, MinNode.create(m1, m2, view), view);
730+
} else if (node instanceof UnsignedMaxNode) {
731+
return UnsignedMaxNode.create(a, UnsignedMaxNode.create(m1, m2, view), view);
732+
} else if (node instanceof UnsignedMinNode) {
733+
return UnsignedMinNode.create(a, UnsignedMinNode.create(m1, m2, view), view);
724734
} else {
725735
throw GraalError.shouldNotReachHere("unhandled node in reassociation with matched values: " + node); // ExcludeFromJacocoGeneratedReport
726736
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/calc/ConditionalNode.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,13 @@ public ValueNode canonical(CanonicalizerTool tool) {
126126
return result;
127127
}
128128

129+
if (tool != null && stamp instanceof IntegerStamp integerStamp && integerStamp.getBits() >= tool.getLowerer().smallestCompareWidth()) {
130+
ValueNode minMaxSynonym = MinMaxNode.fromConditional(condition, trueValue, falseValue, view);
131+
if (minMaxSynonym != null) {
132+
return minMaxSynonym;
133+
}
134+
}
135+
129136
return this;
130137
}
131138

0 commit comments

Comments
 (0)