Skip to content

Commit 9d16ee9

Browse files
davleopodavleopo
committed
[GR-68222] Compile time improvements for large truffle graphs in community.
PullRequest: graal/21762
2 parents 97de41e + 71400a1 commit 9d16ee9

File tree

9 files changed

+179
-38
lines changed

9 files changed

+179
-38
lines changed

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/graph/Graph.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import jdk.graal.compiler.graph.Node.ValueNumberable;
5050
import jdk.graal.compiler.graph.iterators.NodeIterable;
5151
import jdk.graal.compiler.graph.iterators.NodePredicate;
52+
import jdk.graal.compiler.nodes.util.GraphUtil;
5253
import jdk.graal.compiler.options.Option;
5354
import jdk.graal.compiler.options.OptionKey;
5455
import jdk.graal.compiler.options.OptionType;
@@ -83,6 +84,7 @@ private enum FreezeState {
8384
*/
8485
public final boolean verifyGraphs;
8586
public final boolean verifyGraphEdges;
87+
public final boolean verifyKillCFGUnusedNodes;
8688

8789
/**
8890
* Cached actual value of {@link GraalOptions#TrackNodeInsertion} to avoid expensive map lookup
@@ -335,6 +337,7 @@ public Graph(String name, OptionValues options, DebugContext debug, boolean trac
335337

336338
verifyGraphs = Options.VerifyGraalGraphs.getValue(options);
337339
verifyGraphEdges = Options.VerifyGraalGraphEdges.getValue(options);
340+
verifyKillCFGUnusedNodes = GraphUtil.Options.VerifyKillCFGUnusedNodes.getValue(options);
338341

339342
trackNodeInsertion = TrackNodeInsertion.getValue(options);
340343
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/graph/Node.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1902,4 +1902,10 @@ public NodeCycles estimatedNodeCycles() {
19021902
return nodeClass.cycles();
19031903
}
19041904

1905+
/**
1906+
* Special tasks to perform on a node before it is encoded.
1907+
*/
1908+
public void beforeEncode() {
1909+
// intentionally left empty
1910+
}
19051911
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ public void prepare(StructuredGraph graph) {
209209
inliningLogCodec.prepare(graph, this::addObject);
210210
optimizationLogCodec.prepare(graph, this::addObject);
211211
for (Node node : graph.getNodes()) {
212+
node.beforeEncode();
212213
NodeClass<? extends Node> nodeClass = node.getNodeClass();
213214

214215
// Create encoding id for the node class

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

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ public void simplify(SimplifierTool tool) {
198198
killOtherSuccessors(tool, successorIndexAtKey(value().asJavaConstant().asInt()));
199199
} else if (tryOptimizeEnumSwitch(tool)) {
200200
return;
201-
} else if (tryRemoveUnreachableKeys(tool, value().stamp(view))) {
201+
} else if (tryRemoveUnreachableKeys(tool, value().stamp(view), null)) {
202202
return;
203203
} else if (switchTransformationOptimization(tool)) {
204204
return;
@@ -491,7 +491,7 @@ static final class KeyData {
491491
* Remove unreachable keys from the switch based on the stamp of the value, i.e., based on the
492492
* known range of the switch value.
493493
*/
494-
public boolean tryRemoveUnreachableKeys(SimplifierTool tool, Stamp valueStamp) {
494+
public boolean tryRemoveUnreachableKeys(SimplifierTool tool, Stamp valueStamp, EconomicMap<AbstractBeginNode, Stamp> successorStampCache) {
495495
if (!(valueStamp instanceof IntegerStamp)) {
496496
return false;
497497
}
@@ -513,13 +513,31 @@ public boolean tryRemoveUnreachableKeys(SimplifierTool tool, Stamp valueStamp) {
513513
return false;
514514

515515
} else if (newKeyDatas.size() == 0) {
516+
if (successorStampCache != null) {
517+
// Clear all successors from cache, this switch will be removed
518+
for (Node successor : successors) {
519+
if (successor != defaultSuccessor()) {
520+
successorStampCache.removeKey((AbstractBeginNode) successor);
521+
}
522+
}
523+
}
524+
516525
if (tool != null) {
517526
tool.addToWorkList(defaultSuccessor());
518527
}
519528
graph().removeSplitPropagate(this, defaultSuccessor());
520529
return true;
521530

522531
} else {
532+
if (successorStampCache != null) {
533+
// Clear all successors from cache, we have new successors and we need to recompute
534+
// their stamps
535+
for (Node successor : successors) {
536+
if (successor != defaultSuccessor()) {
537+
successorStampCache.removeKey((AbstractBeginNode) successor);
538+
}
539+
}
540+
}
523541
int newDefaultSuccessor = addNewSuccessor(defaultSuccessor(), newSuccessors);
524542
double newDefaultProbability = getKeyProbabilities()[getKeyProbabilities().length - 1];
525543
doReplace(value(), newKeyDatas, newSuccessors, newDefaultSuccessor, newDefaultProbability);
@@ -703,19 +721,12 @@ private void doReplace(ValueNode newValue, List<KeyData> newKeyDatas, ArrayList<
703721
}
704722

705723
@Override
706-
public Stamp getValueStampForSuccessor(AbstractBeginNode beginNode) {
707-
Stamp result = null;
708-
if (beginNode != this.defaultSuccessor()) {
709-
for (int i = 0; i < keyCount(); i++) {
710-
if (keySuccessor(i) == beginNode) {
711-
if (result == null) {
712-
result = StampFactory.forConstant(keyAt(i));
713-
} else {
714-
result = result.meet(StampFactory.forConstant(keyAt(i)));
715-
}
716-
}
717-
}
718-
}
719-
return result;
724+
protected Stamp stampAtKeySuccessor(int i) {
725+
return StampFactory.forConstant(keyAt(i));
726+
}
727+
728+
@Override
729+
public Stamp genericSuccessorStamp() {
730+
return value.stamp(NodeView.DEFAULT);
720731
}
721732
}

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

Lines changed: 91 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535

3636
import java.util.Arrays;
3737

38+
import org.graalvm.collections.EconomicMap;
39+
3840
import jdk.graal.compiler.core.common.type.AbstractPointerStamp;
3941
import jdk.graal.compiler.core.common.type.Stamp;
4042
import jdk.graal.compiler.core.common.type.StampFactory;
@@ -78,6 +80,7 @@ public abstract class SwitchNode extends ControlSplitNode implements Simplifiabl
7880
// do not change the contents of keySuccessors, nor of profileData.getKeyProbabilities()
7981
protected final int[] keySuccessors;
8082
protected SwitchProbabilityData profileData;
83+
protected EconomicMap<AbstractBeginNode, Double> successorProbabilityCache;
8184

8285
/**
8386
* Index of the {@link #successors} field in {@link SwitchNode}'s
@@ -126,17 +129,34 @@ public int getSuccessorCount() {
126129

127130
@Override
128131
public double probability(AbstractBeginNode successor) {
132+
if (successorProbabilityCache != null && successorProbabilityCache.containsKey(successor)) {
133+
double cachedProbability = successorProbabilityCache.get(successor);
134+
assert computeProbability(successor) == cachedProbability : Assertions.errorMessage("Different results for cached versus real probability", cachedProbability,
135+
computeProbability(successor));
136+
return cachedProbability;
137+
}
138+
if (successorProbabilityCache == null) {
139+
successorProbabilityCache = EconomicMap.create();
140+
}
141+
double sum = computeProbability(successor);
142+
successorProbabilityCache.put(successor, sum);
143+
return sum;
144+
}
145+
146+
private double computeProbability(AbstractBeginNode successor) {
129147
double sum = 0;
148+
double[] keyProbabilities = getKeyProbabilities();
130149
for (int i = 0; i < keySuccessors.length; i++) {
131150
if (successors.get(keySuccessors[i]) == successor) {
132-
sum += getKeyProbabilities()[i];
151+
sum += keyProbabilities[i];
133152
}
134153
}
135154
return sum;
136155
}
137156

138157
@Override
139158
public boolean setProbability(AbstractBeginNode successor, BranchProbabilityData successorProfileData) {
159+
invalidateProbabilityCache();
140160
double newProbability = successorProfileData.getDesignatedSuccessorProbability();
141161
assert newProbability <= 1.0 && newProbability >= 0.0 : newProbability;
142162

@@ -171,6 +191,9 @@ public boolean setProbability(AbstractBeginNode successor, BranchProbabilityData
171191
}
172192

173193
public void setProfileData(SwitchProbabilityData profileData) {
194+
if (successorProbabilityCache != null) {
195+
successorProbabilityCache.clear();
196+
}
174197
this.profileData = profileData;
175198
assert assertProbabilities();
176199
}
@@ -246,6 +269,7 @@ public AbstractBeginNode blockSuccessor(int i) {
246269

247270
public void setBlockSuccessor(int i, AbstractBeginNode s) {
248271
successors.set(i, s);
272+
invalidateProbabilityCache();
249273
}
250274

251275
public int blockSuccessorCount() {
@@ -361,8 +385,6 @@ protected void killOtherSuccessors(SimplifierTool tool, int survivingEdge) {
361385
graph().removeSplit(this, blockSuccessor(survivingEdge));
362386
}
363387

364-
public abstract Stamp getValueStampForSuccessor(AbstractBeginNode beginNode);
365-
366388
@Override
367389
public NodeCycles estimatedNodeCycles() {
368390
if (keyCount() == 1) {
@@ -403,4 +425,70 @@ public void simplify(SimplifierTool tool) {
403425
private void tryPullThroughSwitch(SimplifierTool tool) {
404426
GraphUtil.tryDeDuplicateSplitSuccessors(this, tool);
405427
}
428+
429+
@Override
430+
public void beforeEncode() {
431+
// purge the cache, we will build it newly after decoding
432+
successorProbabilityCache = null;
433+
}
434+
435+
private void invalidateProbabilityCache() {
436+
if (successorProbabilityCache != null) {
437+
/*
438+
* Setting the probability of a single successor requires rebalancing the probabilities
439+
* of all other successors to ensure the sum remains consistent (i.e., normalized to
440+
* 1.0). Thus, the entire cache needs invalidation.
441+
*/
442+
successorProbabilityCache.clear();
443+
}
444+
}
445+
446+
public Stamp getValueStampForSuccessor(AbstractBeginNode beginNode, EconomicMap<AbstractBeginNode, Stamp> successorStampCache) {
447+
if (successorStampCache.containsKey(beginNode)) {
448+
Stamp cachedStamp = successorStampCache.get(beginNode);
449+
if (Assertions.assertionsEnabled()) {
450+
Stamp computedStamp = computeSuccessorStamp(beginNode);
451+
assert cachedStamp == null && computedStamp == null || cachedStamp != null && cachedStamp.equals(computedStamp) : Assertions.errorMessage("Stamp must be equal", cachedStamp,
452+
computedStamp);
453+
}
454+
return cachedStamp;
455+
}
456+
Stamp result = computeSuccessorStamp(beginNode);
457+
successorStampCache.put(beginNode, result);
458+
return result;
459+
}
460+
461+
public final Stamp computeSuccessorStamp(AbstractBeginNode beginNode) {
462+
Stamp result = null;
463+
if (beginNode != defaultSuccessor()) {
464+
for (int i = 0; i < keyCount(); i++) {
465+
if (keySuccessor(i) == beginNode) {
466+
if (result == null) {
467+
result = stampAtKeySuccessor(i);
468+
} else {
469+
result = result.meet(stampAtKeySuccessor(i));
470+
}
471+
}
472+
}
473+
}
474+
// if we cannot compute a better stamp use the stamp of the value
475+
return result == null ? genericSuccessorStamp() : result;
476+
}
477+
478+
public abstract Stamp genericSuccessorStamp();
479+
480+
public final void getAllSuccessorValueStamps(EconomicMap<AbstractBeginNode, Stamp> cacheInto) {
481+
for (int i = 0; i < keyCount(); i++) {
482+
AbstractBeginNode beginNode = keySuccessor(i);
483+
if (beginNode != this.defaultSuccessor()) {
484+
if (!cacheInto.containsKey(beginNode)) {
485+
cacheInto.put(beginNode, stampAtKeySuccessor(i));
486+
} else {
487+
cacheInto.put(beginNode, cacheInto.get(beginNode).meet(stampAtKeySuccessor(i)));
488+
}
489+
}
490+
}
491+
}
492+
493+
protected abstract Stamp stampAtKeySuccessor(int i);
406494
}

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

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -223,19 +223,16 @@ public void simplify(SimplifierTool tool) {
223223
}
224224

225225
@Override
226-
public Stamp getValueStampForSuccessor(AbstractBeginNode beginNode) {
227-
Stamp result = null;
228-
if (beginNode != defaultSuccessor()) {
229-
for (int i = 0; i < keyCount(); i++) {
230-
if (keySuccessor(i) == beginNode) {
231-
if (result == null) {
232-
result = StampFactory.objectNonNull(TypeReference.createExactTrusted(typeAt(i)));
233-
} else {
234-
result = result.meet(StampFactory.objectNonNull(TypeReference.createExactTrusted(typeAt(i))));
235-
}
236-
}
237-
}
226+
protected Stamp stampAtKeySuccessor(int i) {
227+
return StampFactory.objectNonNull(TypeReference.createExactTrusted(typeAt(i)));
228+
}
229+
230+
@Override
231+
public Stamp genericSuccessorStamp() {
232+
if (value instanceof LoadHubNode lh) {
233+
return lh.getValue().stamp(NodeView.DEFAULT);
238234
}
239-
return result;
235+
// if we do not see the load hub give up and don't bother computing the default stamp
236+
return null;
240237
}
241238
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/util/GraphUtil.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,9 +281,8 @@ public static void killCFG(FixedNode node) {
281281
EconomicSet<Node> unusedNodes = null;
282282
EconomicSet<Node> unsafeNodes = null;
283283
Graph.NodeEventScope nodeEventScope = null;
284-
OptionValues options = node.getOptions();
285284
boolean verifyGraalGraphEdges = node.graph().verifyGraphEdges;
286-
boolean verifyKillCFGUnusedNodes = GraphUtil.Options.VerifyKillCFGUnusedNodes.getValue(options);
285+
boolean verifyKillCFGUnusedNodes = node.graph().verifyKillCFGUnusedNodes;
287286
if (verifyGraalGraphEdges) {
288287
unsafeNodes = collectUnsafeNodes(node.graph());
289288
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/ConditionalEliminationPhase.java‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,7 @@ public static class Instance implements ControlFlowGraph.RecursiveVisitor<Condit
432432
private final boolean processFieldAccess;
433433
private final List<RebuildPiData> piCache = new ArrayList<>(8);
434434
private final EconomicSet<Pair<Stamp, Stamp>> joinedStamps = EconomicSet.create();
435+
protected EconomicMap<AbstractBeginNode, Stamp> successorStampCache;
435436

436437
/**
437438
* Tests which may be eliminated because post dominating tests to prove a broader condition.
@@ -836,6 +837,18 @@ protected void processNode(Node node) {
836837
introducePisForPhis((MergeNode) node);
837838
}
838839

840+
if (node instanceof SwitchNode switchNode) {
841+
/*
842+
* Since later in this phase we will be visiting all control split successors
843+
* the operation of computing successor stamps for switch nodes can be quite
844+
* costly. Thus, we already compute and cache all eagerly here.
845+
*/
846+
if (successorStampCache == null) {
847+
successorStampCache = EconomicMap.create();
848+
}
849+
switchNode.getAllSuccessorValueStamps(successorStampCache);
850+
}
851+
839852
if (node instanceof AbstractBeginNode) {
840853
if (node instanceof LoopExitNode && graph.isBeforeStage(StageFlag.VALUE_PROXY_REMOVAL)) {
841854
// Condition must not be used down this path.
@@ -1280,7 +1293,10 @@ private static boolean maybeMultipleUsages(ValueNode value) {
12801293
protected void processIntegerSwitch(AbstractBeginNode beginNode, IntegerSwitchNode integerSwitchNode) {
12811294
ValueNode value = integerSwitchNode.value();
12821295
if (maybeMultipleUsages(value)) {
1283-
Stamp stamp = integerSwitchNode.getValueStampForSuccessor(beginNode);
1296+
if (successorStampCache == null) {
1297+
successorStampCache = EconomicMap.create();
1298+
}
1299+
Stamp stamp = integerSwitchNode.getValueStampForSuccessor(beginNode, successorStampCache);
12841300
if (stamp != null) {
12851301
registerNewStamp(value, stamp, beginNode);
12861302
}
@@ -1293,7 +1309,10 @@ protected void processTypeSwitch(AbstractBeginNode beginNode, TypeSwitchNode typ
12931309
LoadHubNode loadHub = (LoadHubNode) hub;
12941310
ValueNode value = loadHub.getValue();
12951311
if (maybeMultipleUsages(value)) {
1296-
Stamp stamp = typeSwitch.getValueStampForSuccessor(beginNode);
1312+
if (successorStampCache == null) {
1313+
successorStampCache = EconomicMap.create();
1314+
}
1315+
Stamp stamp = typeSwitch.getValueStampForSuccessor(beginNode, successorStampCache);
12971316
if (stamp != null) {
12981317
registerNewStamp(value, stamp, beginNode);
12991318
}

0 commit comments

Comments
 (0)