Skip to content

Commit 3fe1e5c

Browse files
davleopodavleopo
committed
[GR-66541] Pull through switch.
PullRequest: graal/21261
2 parents 5368588 + 4356dbe commit 3fe1e5c

File tree

7 files changed

+373
-53
lines changed

7 files changed

+373
-53
lines changed

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/PushThroughIfTest.java‎ renamed to ‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/PullThroughIfTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,15 @@
2424
*/
2525
package jdk.graal.compiler.core.test;
2626

27+
import org.junit.Test;
28+
2729
import jdk.graal.compiler.debug.DebugContext;
2830
import jdk.graal.compiler.nodes.FrameState;
2931
import jdk.graal.compiler.nodes.StructuredGraph;
3032
import jdk.graal.compiler.nodes.StructuredGraph.AllowAssumptions;
3133
import jdk.graal.compiler.nodes.util.GraphUtil;
32-
import org.junit.Test;
3334

34-
public class PushThroughIfTest extends GraalCompilerTest {
35+
public class PullThroughIfTest extends GraalCompilerTest {
3536

3637
public int field1;
3738
public int field2;

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

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
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+
package jdk.graal.compiler.core.test;
26+
27+
import org.junit.Assert;
28+
import org.junit.Test;
29+
30+
import jdk.graal.compiler.api.directives.GraalDirectives;
31+
import jdk.graal.compiler.debug.GraalError;
32+
import jdk.graal.compiler.nodes.FixedNode;
33+
import jdk.graal.compiler.nodes.StructuredGraph;
34+
import jdk.graal.compiler.nodes.extended.SwitchNode;
35+
import jdk.graal.compiler.nodes.util.GraphUtil;
36+
37+
public class PullThroughSwitchTest extends GraalCompilerTest {
38+
39+
static int sideEffect;
40+
static int sideEffect1;
41+
static int sideEffect2;
42+
static int sideEffect3;
43+
44+
public static int switchReducePattern(int a) {
45+
int result = 0;
46+
switch (a) {
47+
case 1:
48+
result = sideEffect;
49+
break;
50+
case 2:
51+
result = sideEffect;
52+
break;
53+
case 3:
54+
result = sideEffect;
55+
break;
56+
default:
57+
result = sideEffect;
58+
break;
59+
60+
}
61+
return result;
62+
}
63+
64+
@Test
65+
public void test01() {
66+
highTierSwitches = 0;
67+
test("switchReducePattern", 12);
68+
highTierSwitches = -1;
69+
}
70+
71+
public static int switchNotReducePattern(int a) {
72+
int result = 0;
73+
switch (a) {
74+
case 1:
75+
result = sideEffect;
76+
break;
77+
case 2:
78+
result = sideEffect1;
79+
break;
80+
case 3:
81+
result = sideEffect2;
82+
break;
83+
default:
84+
result = sideEffect3;
85+
break;
86+
87+
}
88+
return result;
89+
}
90+
91+
@Test
92+
public void test02() {
93+
highTierSwitches = 1;
94+
test("switchNotReducePattern", 12);
95+
highTierSwitches = -1;
96+
}
97+
98+
public static int switchReduceOnlyOneNodePattern(int a) {
99+
int result = 0;
100+
int result1 = 0;
101+
switch (a) {
102+
case 1:
103+
result = sideEffect;
104+
result1 = sideEffect1;
105+
break;
106+
case 2:
107+
result = sideEffect;
108+
result1 = sideEffect2;
109+
break;
110+
case 3:
111+
result = sideEffect;
112+
result1 = sideEffect3;
113+
break;
114+
default:
115+
result = sideEffect;
116+
result1 = sideEffect1;
117+
break;
118+
119+
}
120+
return result + result1;
121+
}
122+
123+
@Test
124+
public void test03() {
125+
highTierSwitches = 1;
126+
// we only deduplicated exactly 1 node
127+
fixedNodesBeforeSwitch = 1;
128+
test("switchReduceOnlyOneNodePattern", 12);
129+
highTierSwitches = -1;
130+
fixedNodesBeforeSwitch = -1;
131+
}
132+
133+
public static int switchReduce3NodesPattern(int a) {
134+
int result = 0;
135+
int result1 = 0;
136+
int result2 = 0;
137+
int result3 = 0;
138+
switch (a) {
139+
case 1:
140+
result = sideEffect;
141+
result1 = sideEffect1;
142+
result2 = sideEffect2;
143+
result3 = sideEffect3;
144+
GraalDirectives.sideEffect(1);
145+
break;
146+
case 2:
147+
result = sideEffect;
148+
result1 = sideEffect1;
149+
result2 = sideEffect2;
150+
result3 = sideEffect3;
151+
GraalDirectives.sideEffect(2);
152+
break;
153+
case 3:
154+
result = sideEffect;
155+
result1 = sideEffect1;
156+
result2 = sideEffect2;
157+
result3 = sideEffect3;
158+
GraalDirectives.sideEffect(3);
159+
break;
160+
default:
161+
result = sideEffect;
162+
result1 = sideEffect1;
163+
result2 = sideEffect2;
164+
result3 = sideEffect3;
165+
GraalDirectives.sideEffect(4);
166+
break;
167+
168+
}
169+
return result + result1 + result2 + result3;
170+
}
171+
172+
@Test
173+
public void test04() {
174+
highTierSwitches = 1;
175+
// we only deduplicated exactly 1 node
176+
fixedNodesBeforeSwitch = 4;
177+
test("switchReduce3NodesPattern", 12);
178+
highTierSwitches = -1;
179+
fixedNodesBeforeSwitch = -1;
180+
}
181+
182+
// default value is -1
183+
private int highTierSwitches = -1;
184+
185+
// nodes before the switch until start, excluding start, only checked if >=0
186+
private int fixedNodesBeforeSwitch = -1;
187+
188+
@Override
189+
protected void checkHighTierGraph(StructuredGraph graph) {
190+
Assert.assertEquals("Must have that many switches left", graph.getNodes().filter(SwitchNode.class).count(), highTierSwitches);
191+
if (fixedNodesBeforeSwitch >= 0) {
192+
checkBeforeNodes: {
193+
SwitchNode sw = graph.getNodes().filter(SwitchNode.class).first();
194+
int fixedNodes = 0;
195+
for (FixedNode f : GraphUtil.predecessorIterable((FixedNode) sw.predecessor())) {
196+
if (f == graph.start()) {
197+
Assert.assertEquals("Must have exactly that many fixed nodes left before switch", fixedNodesBeforeSwitch, fixedNodes);
198+
break checkBeforeNodes;
199+
}
200+
fixedNodes++;
201+
}
202+
throw GraalError.shouldNotReachHere("Must find start node, no other basic block before");
203+
}
204+
}
205+
}
206+
207+
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/IfNode.java‎

Lines changed: 3 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import jdk.graal.compiler.core.common.type.PrimitiveStamp;
4242
import jdk.graal.compiler.core.common.type.Stamp;
4343
import jdk.graal.compiler.core.common.type.StampFactory;
44-
import jdk.graal.compiler.core.common.util.CompilationAlarm;
4544
import jdk.graal.compiler.debug.Assertions;
4645
import jdk.graal.compiler.debug.CounterKey;
4746
import jdk.graal.compiler.debug.DebugCloseable;
@@ -71,12 +70,10 @@
7170
import jdk.graal.compiler.nodes.calc.ObjectEqualsNode;
7271
import jdk.graal.compiler.nodes.calc.SubNode;
7372
import jdk.graal.compiler.nodes.cfg.HIRBlock;
74-
import jdk.graal.compiler.nodes.debug.ControlFlowAnchored;
7573
import jdk.graal.compiler.nodes.extended.BranchProbabilityNode;
7674
import jdk.graal.compiler.nodes.extended.UnboxNode;
7775
import jdk.graal.compiler.nodes.java.InstanceOfNode;
7876
import jdk.graal.compiler.nodes.java.LoadFieldNode;
79-
import jdk.graal.compiler.nodes.memory.MemoryAnchorNode;
8077
import jdk.graal.compiler.nodes.spi.Canonicalizable;
8178
import jdk.graal.compiler.nodes.spi.LIRLowerable;
8279
import jdk.graal.compiler.nodes.spi.NodeLIRBuilderTool;
@@ -260,7 +257,7 @@ public void simplify(SimplifierTool tool) {
260257
}
261258
if (tool.allUsagesAvailable() && trueSuccessor().hasNoUsages() && falseSuccessor().hasNoUsages()) {
262259

263-
pushNodesThroughIf(tool);
260+
pullNodesThroughIf(tool);
264261

265262
if (checkForUnsignedCompare(tool) || removeOrMaterializeIf(tool)) {
266263
return;
@@ -1157,54 +1154,10 @@ private boolean conditionalNodeOptimization(SimplifierTool tool) {
11571154
return false;
11581155
}
11591156

1160-
private void pushNodesThroughIf(SimplifierTool tool) {
1157+
private void pullNodesThroughIf(SimplifierTool tool) {
11611158
assert trueSuccessor().hasNoUsages() : Assertions.errorMessageContext("this", this, "trueSucc", trueSuccessor(), "trueSuccUsages", trueSuccessor().usages());
11621159
assert falseSuccessor().hasNoUsages() : Assertions.errorMessageContext("this", this, "falseSucc", falseSuccessor(), "falseSuccUsages", falseSuccessor().usages());
1163-
// push similar nodes upwards through the if, thereby deduplicating them
1164-
do {
1165-
CompilationAlarm.checkProgress(graph());
1166-
AbstractBeginNode trueSucc = trueSuccessor();
1167-
AbstractBeginNode falseSucc = falseSuccessor();
1168-
if (trueSucc instanceof BeginNode && falseSucc instanceof BeginNode && trueSucc.next() instanceof FixedWithNextNode && falseSucc.next() instanceof FixedWithNextNode) {
1169-
FixedWithNextNode trueNext = (FixedWithNextNode) trueSucc.next();
1170-
FixedWithNextNode falseNext = (FixedWithNextNode) falseSucc.next();
1171-
NodeClass<?> nodeClass = trueNext.getNodeClass();
1172-
if (trueNext.getClass() == falseNext.getClass()) {
1173-
if (trueNext instanceof AbstractBeginNode || trueNext instanceof ControlFlowAnchored || trueNext instanceof MemoryAnchorNode) {
1174-
/*
1175-
* Cannot do this optimization for begin nodes, because it could move guards
1176-
* above the if that need to stay below a branch.
1177-
*
1178-
* Cannot do this optimization for ControlFlowAnchored nodes, because these
1179-
* are anchored in their control-flow position, and should not be moved
1180-
* upwards.
1181-
*/
1182-
} else if (nodeClass.equalInputs(trueNext, falseNext) && trueNext.valueEquals(falseNext)) {
1183-
falseNext.replaceAtUsages(trueNext);
1184-
graph().removeFixed(falseNext);
1185-
GraphUtil.unlinkFixedNode(trueNext);
1186-
graph().addBeforeFixed(this, trueNext);
1187-
for (Node usage : trueNext.usages().snapshot()) {
1188-
if (usage.isAlive()) {
1189-
NodeClass<?> usageNodeClass = usage.getNodeClass();
1190-
if (usageNodeClass.valueNumberable() && !usageNodeClass.isLeafNode()) {
1191-
Node newNode = graph().findDuplicate(usage);
1192-
if (newNode != null) {
1193-
usage.replaceAtUsagesAndDelete(newNode);
1194-
}
1195-
}
1196-
if (usage.isAlive()) {
1197-
tool.addToWorkList(usage);
1198-
}
1199-
}
1200-
}
1201-
continue;
1202-
}
1203-
}
1204-
}
1205-
break;
1206-
} while (true); // TERMINATION ARGUMENT: processing fixed nodes until duplication is no
1207-
// longer possible.
1160+
GraphUtil.tryDeDuplicateSplitSuccessors(this, tool);
12081161
}
12091162

12101163
/**

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/IntegerSwitchNode.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,10 @@ public int successorIndexAtKey(int key) {
183183

184184
@Override
185185
public void simplify(SimplifierTool tool) {
186+
super.simplify(tool);
187+
if (this.isDeleted()) {
188+
return;
189+
}
186190
if (shouldInjectBranchProbabilities()) {
187191
injectBranchProbabilities();
188192
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/SwitchNode.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@
5353
import jdk.graal.compiler.nodes.ProfileData.BranchProbabilityData;
5454
import jdk.graal.compiler.nodes.ProfileData.SwitchProbabilityData;
5555
import jdk.graal.compiler.nodes.ValueNode;
56+
import jdk.graal.compiler.nodes.spi.Simplifiable;
5657
import jdk.graal.compiler.nodes.spi.SimplifierTool;
58+
import jdk.graal.compiler.nodes.util.GraphUtil;
5759
import jdk.vm.ci.meta.Constant;
5860

5961
/**
@@ -67,7 +69,7 @@
6769
sizeRationale = "We cannot estimate the code size of a switch statement without knowing the number" +
6870
"of case statements.")
6971
// @formatter:on
70-
public abstract class SwitchNode extends ControlSplitNode {
72+
public abstract class SwitchNode extends ControlSplitNode implements Simplifiable {
7173

7274
public static final NodeClass<SwitchNode> TYPE = NodeClass.create(SwitchNode.class);
7375
@Successor protected NodeSuccessorList<AbstractBeginNode> successors;
@@ -392,4 +394,13 @@ protected NodeSize dynamicNodeSizeEstimate() {
392394
public int[] getKeySuccessors() {
393395
return keySuccessors.clone();
394396
}
397+
398+
@Override
399+
public void simplify(SimplifierTool tool) {
400+
tryPullThroughSwitch(tool);
401+
}
402+
403+
private void tryPullThroughSwitch(SimplifierTool tool) {
404+
GraphUtil.tryDeDuplicateSplitSuccessors(this, tool);
405+
}
395406
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/java/TypeSwitchNode.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@ public void generate(NodeLIRBuilderTool gen) {
128128

129129
@Override
130130
public void simplify(SimplifierTool tool) {
131+
super.simplify(tool);
132+
if (this.isDeleted()) {
133+
return;
134+
}
131135
if (shouldInjectBranchProbabilities()) {
132136
injectBranchProbabilities();
133137
}

0 commit comments

Comments
 (0)