From e9e73c73709b2af076af8f662d72debf95cf0d30 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Mon, 13 Oct 2025 13:05:09 +0200 Subject: [PATCH 1/9] control flow graph: make dom tree visit decorator generic --- .../jdk/graal/compiler/nodes/cfg/ControlFlowGraph.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/cfg/ControlFlowGraph.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/cfg/ControlFlowGraph.java index 14ed0b80c3b8..d48ee50d867f 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/cfg/ControlFlowGraph.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/cfg/ControlFlowGraph.java @@ -448,11 +448,11 @@ public void updateCachedLocalLoopFrequency(LoopBeginNode lb, Function { - private final ControlFlowGraph.RecursiveVisitor visitor; + public static class LoggingCFGDecorator implements ControlFlowGraph.RecursiveVisitor { + private final ControlFlowGraph.RecursiveVisitor visitor; private String indent = ""; - public LoggingCFGDecorator(ControlFlowGraph.RecursiveVisitor visitor, ControlFlowGraph cfg) { + public LoggingCFGDecorator(ControlFlowGraph.RecursiveVisitor visitor, ControlFlowGraph cfg) { this.visitor = visitor; TTY.printf("DomTree for %s%n", cfg.graph); printDomTree(cfg.getStartBlock(), ""); @@ -468,14 +468,14 @@ private static void printDomTree(HIRBlock cur, String indent) { } @Override - public HIRBlock enter(HIRBlock b) { + public L enter(HIRBlock b) { TTY.printf("%sEnter block %s for %s%n", indent, b, visitor); indent += "\t"; return visitor.enter(b); } @Override - public void exit(HIRBlock b, HIRBlock value) { + public void exit(HIRBlock b, L value) { indent = indent.substring(0, indent.length() - 1); TTY.printf("%sExit block %s with value %s for %s%n", indent, b, value, visitor); visitor.exit(b, value); From f9346dababa2ee0ad21532d510fa02d709c1665e Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Mon, 13 Oct 2025 13:05:40 +0200 Subject: [PATCH 2/9] shared arena support: optimize away early if arena is null --- .../foreign/MemoryArenaValidInScopeNode.java | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java index 597023197131..bf99103f7227 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java @@ -28,6 +28,7 @@ import java.lang.reflect.Field; +import org.graalvm.collections.EconomicSet; import org.graalvm.word.LocationIdentity; import com.oracle.svm.core.nodes.ClusterNode; @@ -41,11 +42,14 @@ import jdk.graal.compiler.nodeinfo.NodeInfo; import jdk.graal.compiler.nodeinfo.NodeSize; import jdk.graal.compiler.nodes.ConstantNode; +import jdk.graal.compiler.nodes.FixedNode; import jdk.graal.compiler.nodes.FixedWithNextNode; import jdk.graal.compiler.nodes.ValueNode; import jdk.graal.compiler.nodes.debug.ControlFlowAnchored; import jdk.graal.compiler.nodes.memory.MemoryAccess; import jdk.graal.compiler.nodes.memory.MemoryKill; +import jdk.graal.compiler.nodes.spi.Simplifiable; +import jdk.graal.compiler.nodes.spi.SimplifierTool; import jdk.internal.foreign.MemorySessionImpl; import jdk.internal.misc.ScopedMemoryAccess; @@ -55,7 +59,7 @@ * Mark the beginning of a {@link ScopedMemoryAccess} checking the validity of a memory session. */ @NodeInfo(cycles = NodeCycles.CYCLES_UNKNOWN, size = NodeSize.SIZE_UNKNOWN) -public class MemoryArenaValidInScopeNode extends FixedWithNextNode implements MemoryAccess, ControlFlowAnchored { +public class MemoryArenaValidInScopeNode extends FixedWithNextNode implements MemoryAccess, ControlFlowAnchored, Simplifiable { public static final Field STATE_FIELD = ReflectionUtil.lookupField(MemorySessionImpl.class, "state"); public static final NodeClass TYPE = NodeClass.create(MemoryArenaValidInScopeNode.class); @@ -103,4 +107,50 @@ public LocationIdentity getLocationIdentity() { return fieldLocation; } + @Override + public void simplify(SimplifierTool tool) { + if (tool.allUsagesAvailable() && graph() != null) { + ValueNode session = value; + if (value.isConstant() && value.isNullConstant()) { + FixedNode predecessor = (FixedNode) this.predecessor(); + + EconomicSet scopeNodes = EconomicSet.create(); + // also cleanup any other nodes that check for inlined callees + // this is a best effort + FixedNode cur = predecessor; + while (cur != null) { + if (cur instanceof ScopedMethodNode scopeNode) { + // scope nodes are always balanced, find the next start, that must be ours, + // if we find an end we do not delete our pair + if (scopeNode.getType() == ScopedMethodNode.Type.START) { + scopeNodes.add(scopeNode); + for (Node usage : scopeNode.usages()) { + if (usage instanceof ScopedMethodNode scopeUsage && scopeUsage.getType() == ScopedMethodNode.Type.END && scopeUsage.getStart() == scopeNode) { + scopeNodes.add(scopeUsage); + } + } + // we collected the usages, check that we found all of them + for (Node usage : scopeNode.usages()) { + if (!(usage instanceof ScopedMethodNode sm && scopeNodes.contains(sm))) { + return; + } + } + break; + } else { + return; + } + } + cur = (FixedNode) cur.predecessor(); + } + + // delete and replace the scope node first + this.delete(0); + + // clear all the scope nodes + for (ScopedMethodNode scopeNode : scopeNodes) { + scopeNode.delete(); + } + } + } + } } From 845a8fde2fb721bee035bbe1016eb212f1fc8f8d Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Mon, 13 Oct 2025 13:06:11 +0200 Subject: [PATCH 3/9] shared arena support: several fixes including: only process nodes in arena methods (scopes must not be empty), handle scope open close in same basic block, handle empty arena regions --- ...bstrateOptimizeSharedArenaAccessPhase.java | 68 ++++++++++++++----- 1 file changed, 52 insertions(+), 16 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/phases/SubstrateOptimizeSharedArenaAccessPhase.java b/substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/phases/SubstrateOptimizeSharedArenaAccessPhase.java index af07cf1713c6..11d8ffb4e9c6 100644 --- a/substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/phases/SubstrateOptimizeSharedArenaAccessPhase.java +++ b/substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/phases/SubstrateOptimizeSharedArenaAccessPhase.java @@ -779,6 +779,9 @@ private void cleanupClusterNodes(StructuredGraph graph, MidTierContext context, clusterNode.delete(); } } + + graph.getDebug().dump(DebugContext.VERY_DETAILED_LEVEL, graph, "Before running final canonicalization"); + canonicalizer.apply(graph, context); scheduleVerify(graph); @@ -878,21 +881,48 @@ private static EconomicMap> enumerateScopedAccesses(Opt @Override public Integer enter(HIRBlock b) { - int newDominatingValues = 0; - int newScopesToPop = 0; + // all new nodes for checking arenas, pop if we go out of scope of this block again + int newArinaValidInScopeNodes = 0; + // all scopes that are closed in this block that have been open before already Deque scopesToRepush = new ArrayDeque<>(); + EconomicSet newOpenedScopes = EconomicSet.create(); + for (FixedNode f : b.getNodes()) { if (f instanceof MemoryArenaValidInScopeNode mas) { defs.push(new ReachingDefScope(mas)); - newDominatingValues++; + newArinaValidInScopeNodes++; } else if (f instanceof ScopedMethodNode scope) { + /* + * When we process scope nodes we have 3 major situations to deal with. + * + * no open scope, no scopes openend in "this" block -> do nothing + * + * we open a new scope s, we do not close it -> pop it when exit() is called + * + * we close a scope that we have not opened -> pop it and repush on exit() + * + * we open and close the same new scope in this block -> do nothing on + * exit() + */ + if (scope.getType() == ScopedMethodNode.Type.START) { scopes.push(scope); - newScopesToPop++; + // we open a new scope, we have to pop it again if this block goes out + // of scope + newOpenedScopes.add(scope); } else if (scope.getType() == ScopedMethodNode.Type.END) { ScopedMethodNode start = scopes.pop(); - scopesToRepush.push(start); + // we close a scope, we only have to repush it again in the dom tree + // traversal if it is a scope that was open upon the enter call to the + // current block + if (!newOpenedScopes.contains(start)) { + scopesToRepush.push(start); + } else { + // remove it again from the opened scopes, it is already closed + // again + newOpenedScopes.remove(start); + } assert scope.getStart() == start : Assertions.errorMessage("Must match", start, scope, scope.getStart()); } else { throw GraalError.shouldNotReachHere("Unknown type " + scope.getType()); @@ -902,8 +932,10 @@ public Integer enter(HIRBlock b) { } } - final int finalNewDominatingValues = newDominatingValues; - final int finalNewScopesToPop = newScopesToPop; + final int finalNewDominatingValues = newArinaValidInScopeNodes; + // all new scopes that have not been closed already need to be removed again when we + // go out of this block + final int finalNewScopesToPop = newOpenedScopes.size(); actions.push(new Runnable() { @@ -925,8 +957,20 @@ public void run() { return 1; } + @Override + public void exit(HIRBlock b, Integer pushedForBlock) { + for (int i = 0; i < pushedForBlock; i++) { + actions.pop().run(); + } + } + private void processNode(FixedNode f) { - if (!scopes.isEmpty() && f instanceof Invoke i) { + if (scopes.isEmpty()) { + // no open scopes, nothing to check, we are not inside an SharedArena access + // method. + return; + } + if (f instanceof Invoke i) { if (i.getTargetMethod() != null && !config.isSafeCallee(i.getTargetMethod())) { if (!defs.isEmpty()) { dominatedCalls.add(new DominatedCall(defs.peek().defNode, i)); @@ -1055,15 +1099,7 @@ private EconomicSet> visitEveryLoopHeaderInBetween(ReachingDef return loopsToCheck; } - @Override - public void exit(HIRBlock b, Integer pushedForBlock) { - for (int i = 0; i < pushedForBlock; i++) { - actions.pop().run(); - } - } - }; - cfg.visitDominatorTreeDefault(visitor); return nodeAccesses.size() > 0 ? nodeAccesses : null; } From 9dbfe520bf848d9d14b077387db60c5cac18f2ee Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Tue, 14 Oct 2025 09:45:37 +0200 Subject: [PATCH 4/9] svm shared arena support: tie the control flow anchor for the session exception handler code to the actual existance of the scope checking, if that is optimized away, also fold the anchor --- .../api/directives/GraalDirectives.java | 7 ++++++ .../nodes/debug/ControlFlowAnchorNode.java | 24 ++++++++++++++++++- .../StandardGraphBuilderPlugins.java | 9 ++++++- .../core/foreign/SubstrateForeignUtil.java | 2 +- .../foreign/MemoryArenaValidInScopeNode.java | 4 ++-- 5 files changed, 41 insertions(+), 5 deletions(-) diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/api/directives/GraalDirectives.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/api/directives/GraalDirectives.java index 135febd66e61..09d7dbcc5d92 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/api/directives/GraalDirectives.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/api/directives/GraalDirectives.java @@ -157,6 +157,13 @@ public static boolean inIntrinsic() { public static void controlFlowAnchor() { } + /** + * Like {@link #controlFlowAnchor()} except this node can be optimized away if its + * {@code condition} argument becomes constant {@code 0}. + */ + public static void controlFlowAnchor(long condition) { + } + /** * A call to this method will disable strip mining of the enclosing loop in the compiler. */ diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/debug/ControlFlowAnchorNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/debug/ControlFlowAnchorNode.java index 39a33f3d0512..d03d3d83cc24 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/debug/ControlFlowAnchorNode.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/debug/ControlFlowAnchorNode.java @@ -33,18 +33,28 @@ import jdk.graal.compiler.nodeinfo.NodeInfo; import jdk.graal.compiler.nodes.FixedWithNextNode; import jdk.graal.compiler.nodes.Invoke; +import jdk.graal.compiler.nodes.ValueNode; +import jdk.graal.compiler.nodes.spi.Canonicalizable; +import jdk.graal.compiler.nodes.spi.CanonicalizerTool; import jdk.graal.compiler.nodes.spi.LIRLowerable; import jdk.graal.compiler.nodes.spi.NodeLIRBuilderTool; @NodeInfo(cycles = CYCLES_0, size = SIZE_0) -public final class ControlFlowAnchorNode extends FixedWithNextNode implements LIRLowerable, ControlFlowAnchored { +public final class ControlFlowAnchorNode extends FixedWithNextNode implements LIRLowerable, ControlFlowAnchored, Canonicalizable { public static final NodeClass TYPE = NodeClass.create(ControlFlowAnchorNode.class); + @OptionalInput protected ValueNode numericCondition; + public ControlFlowAnchorNode() { super(TYPE, StampFactory.forVoid()); } + public ControlFlowAnchorNode(ValueNode numericCondition) { + this(); + this.numericCondition = numericCondition; + } + /** * Used by MacroSubstitution. */ @@ -61,4 +71,16 @@ public void generate(NodeLIRBuilderTool generator) { protected void afterClone(Node other) { assert other.graph() != null && other.graph() != graph() : this + " should never be cloned in the same graph"; } + + @Override + public Node canonical(CanonicalizerTool tool) { + if (tool.allUsagesAvailable() && numericCondition != null) { + // if we have a condition that evals to 0 + if (numericCondition.isConstant() && numericCondition.asJavaConstant().asLong() == 0L) { + // delete this node + return null; + } + } + return this; + } } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java index ad4adfd34960..df20a31851eb 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java @@ -49,7 +49,6 @@ import java.util.Objects; import java.util.function.BiFunction; -import jdk.graal.compiler.replacements.nodes.ThreadedSwitchNode; import org.graalvm.word.LocationIdentity; import jdk.graal.compiler.api.directives.GraalDirectives; @@ -213,6 +212,7 @@ import jdk.graal.compiler.replacements.nodes.ProfileBooleanNode; import jdk.graal.compiler.replacements.nodes.ReverseBitsNode; import jdk.graal.compiler.replacements.nodes.ReverseBytesNode; +import jdk.graal.compiler.replacements.nodes.ThreadedSwitchNode; import jdk.graal.compiler.replacements.nodes.VectorizedHashCodeNode; import jdk.graal.compiler.replacements.nodes.VectorizedMismatchNode; import jdk.graal.compiler.replacements.nodes.VirtualizableInvokeMacroNode; @@ -1788,6 +1788,13 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec return true; } }); + r.register(new RequiredInlineOnlyInvocationPlugin("controlFlowAnchor", long.class) { + @Override + public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode condition) { + b.add(new ControlFlowAnchorNode(condition)); + return true; + } + }); r.register(new RequiredInlineOnlyInvocationPlugin("neverStripMine") { @Override public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver) { diff --git a/substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/SubstrateForeignUtil.java b/substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/SubstrateForeignUtil.java index 960442874d7e..de7dfd028844 100644 --- a/substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/SubstrateForeignUtil.java +++ b/substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/SubstrateForeignUtil.java @@ -117,6 +117,6 @@ public static void sessionExceptionHandler(MemorySessionImpl session, Object bas } // avoid any optimization based code duplication in the cluster, it disrupts later matching // of control flow when searching for this pattern - GraalDirectives.controlFlowAnchor(); + GraalDirectives.controlFlowAnchor(scope); } } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java index bf99103f7227..3b50247136d5 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java @@ -110,8 +110,8 @@ public LocationIdentity getLocationIdentity() { @Override public void simplify(SimplifierTool tool) { if (tool.allUsagesAvailable() && graph() != null) { - ValueNode session = value; - if (value.isConstant() && value.isNullConstant()) { + final ValueNode session = value; + if (session.isConstant() && session.isNullConstant()) { FixedNode predecessor = (FixedNode) this.predecessor(); EconomicSet scopeNodes = EconomicSet.create(); From 361ec28a1c8b04f5bbcb4b46dfce8a8b99f1d8e9 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Tue, 14 Oct 2025 12:27:09 +0200 Subject: [PATCH 5/9] suppress unused warning --- .../src/jdk/graal/compiler/api/directives/GraalDirectives.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/api/directives/GraalDirectives.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/api/directives/GraalDirectives.java index 09d7dbcc5d92..caf21488c319 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/api/directives/GraalDirectives.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/api/directives/GraalDirectives.java @@ -161,7 +161,7 @@ public static void controlFlowAnchor() { * Like {@link #controlFlowAnchor()} except this node can be optimized away if its * {@code condition} argument becomes constant {@code 0}. */ - public static void controlFlowAnchor(long condition) { + public static void controlFlowAnchor(@SuppressWarnings("unused") long condition) { } /** From 0a2bcdf92b5511b68acab05af843488d9f75e8b9 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Wed, 15 Oct 2025 08:53:43 +0200 Subject: [PATCH 6/9] shared arena support: optimize scope and valid node in the same batch --- .../nodes/foreign/MemoryArenaValidInScopeNode.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java index 3b50247136d5..552bd8367fb9 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java @@ -143,6 +143,16 @@ public void simplify(SimplifierTool tool) { cur = (FixedNode) cur.predecessor(); } + if (scopeNodes.isEmpty()) { + /* + * Collect and delete all scope nodes in a single batch to ensure they are + * optimized together. If we don't do this, the current node might be optimized + * away before the scope nodes are inlined, resulting in incomplete + * optimization. We wait until both are present in the graph before optimizing. + */ + return; + } + // delete and replace the scope node first this.delete(0); From 63bc1782c5f33a32c4dc01fbab09f98e830962c7 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Thu, 16 Oct 2025 08:02:14 +0200 Subject: [PATCH 7/9] checkstyle fixes --- .../phases/SubstrateOptimizeSharedArenaAccessPhase.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/phases/SubstrateOptimizeSharedArenaAccessPhase.java b/substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/phases/SubstrateOptimizeSharedArenaAccessPhase.java index 11d8ffb4e9c6..b2c0102b458b 100644 --- a/substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/phases/SubstrateOptimizeSharedArenaAccessPhase.java +++ b/substratevm/src/com.oracle.svm.core.foreign/src/com/oracle/svm/core/foreign/phases/SubstrateOptimizeSharedArenaAccessPhase.java @@ -882,7 +882,7 @@ private static EconomicMap> enumerateScopedAccesses(Opt @Override public Integer enter(HIRBlock b) { // all new nodes for checking arenas, pop if we go out of scope of this block again - int newArinaValidInScopeNodes = 0; + int newArenaValidInScopeNodes = 0; // all scopes that are closed in this block that have been open before already Deque scopesToRepush = new ArrayDeque<>(); @@ -891,12 +891,12 @@ public Integer enter(HIRBlock b) { for (FixedNode f : b.getNodes()) { if (f instanceof MemoryArenaValidInScopeNode mas) { defs.push(new ReachingDefScope(mas)); - newArinaValidInScopeNodes++; + newArenaValidInScopeNodes++; } else if (f instanceof ScopedMethodNode scope) { /* * When we process scope nodes we have 3 major situations to deal with. * - * no open scope, no scopes openend in "this" block -> do nothing + * no open scope, no scopes opened in "this" block -> do nothing * * we open a new scope s, we do not close it -> pop it when exit() is called * @@ -932,7 +932,7 @@ public Integer enter(HIRBlock b) { } } - final int finalNewDominatingValues = newArinaValidInScopeNodes; + final int finalNewDominatingValues = newArenaValidInScopeNodes; // all new scopes that have not been closed already need to be removed again when we // go out of this block final int finalNewScopesToPop = newOpenedScopes.size(); From c76f4b4ace62b35121598a2b95f13933693f4a44 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Thu, 16 Oct 2025 08:02:40 +0200 Subject: [PATCH 8/9] add cfg decorator test to illustrate usage --- .../core/test/LoggingCFGDecoratorTest.java | 96 +++++++++++++++++++ .../compiler/nodes/cfg/ControlFlowGraph.java | 22 +++-- 2 files changed, 109 insertions(+), 9 deletions(-) create mode 100644 compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/LoggingCFGDecoratorTest.java diff --git a/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/LoggingCFGDecoratorTest.java b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/LoggingCFGDecoratorTest.java new file mode 100644 index 000000000000..45f2bc780d93 --- /dev/null +++ b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/LoggingCFGDecoratorTest.java @@ -0,0 +1,96 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package jdk.graal.compiler.core.test; + +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; + +import org.junit.Test; + +import jdk.graal.compiler.api.directives.GraalDirectives; +import jdk.graal.compiler.nodes.StructuredGraph; +import jdk.graal.compiler.nodes.cfg.ControlFlowGraph; +import jdk.graal.compiler.nodes.cfg.HIRBlock; + +public class LoggingCFGDecoratorTest extends GraalCompilerTest { + + public static int foo(int a) { + int result = 0; + if (a == 0) { + result = 1; + } else { + result = 2; + } + GraalDirectives.controlFlowAnchor(); + ; + return result; + } + + @Test + public void test01() { + StructuredGraph g = parseEager("foo", StructuredGraph.AllowAssumptions.NO); + ControlFlowGraph cfg = ControlFlowGraph.computeForSchedule(g); + ControlFlowGraph.RecursiveVisitor visitor = new ControlFlowGraph.RecursiveVisitor() { + int number; + + @Override + public Integer enter(HIRBlock b) { + number++; + // pushed elements + return 1; + } + + @Override + public void exit(HIRBlock b, Integer value) { + number -= value; + } + + @Override + public String toString() { + return "TestIterator"; + } + }; + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintStream o = new PrintStream(baos); + ControlFlowGraph.LoggingCFGDecorator dec = new ControlFlowGraph.LoggingCFGDecorator<>(o, visitor, cfg); + cfg.visitDominatorTreeDefault(dec); + String result = baos.toString(); + assert result.contains(ExpectedOutput); + } + + private static final String ExpectedOutput = "B0 [dom null, post dom null]\n" + + "\tB1 [dom B0, post dom null]\n" + + "\tB2 [dom B0, post dom null]\n" + + "\tB3 [dom B0, post dom null]\n" + + "Enter block B0 for TestIterator\n" + + "\tEnter block B1 for TestIterator\n" + + "\tExit block B1 with value 1 for TestIterator\n" + + "\tEnter block B2 for TestIterator\n" + + "\tExit block B2 with value 1 for TestIterator\n" + + "\tEnter block B3 for TestIterator\n" + + "\tExit block B3 with value 1 for TestIterator\n" + + "Exit block B0 with value 1 for TestIterator"; + +} diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/cfg/ControlFlowGraph.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/cfg/ControlFlowGraph.java index d48ee50d867f..4b7626e43d21 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/cfg/ControlFlowGraph.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/cfg/ControlFlowGraph.java @@ -24,6 +24,7 @@ */ package jdk.graal.compiler.nodes.cfg; +import java.io.PrintStream; import java.util.ArrayList; import java.util.List; import java.util.function.Consumer; @@ -446,30 +447,33 @@ public void updateCachedLocalLoopFrequency(LoopBeginNode lb, Function implements ControlFlowGraph.RecursiveVisitor { private final ControlFlowGraph.RecursiveVisitor visitor; private String indent = ""; + private final PrintStream out; - public LoggingCFGDecorator(ControlFlowGraph.RecursiveVisitor visitor, ControlFlowGraph cfg) { + public LoggingCFGDecorator(PrintStream out, ControlFlowGraph.RecursiveVisitor visitor, ControlFlowGraph cfg) { this.visitor = visitor; - TTY.printf("DomTree for %s%n", cfg.graph); - printDomTree(cfg.getStartBlock(), ""); + this.out = out; + out.printf("DomTree for %s%n", cfg.graph); + printDomTree(out, cfg.getStartBlock(), ""); } - private static void printDomTree(HIRBlock cur, String indent) { - TTY.printf("%s%s [dom %s, post dom %s]%n", indent, cur, cur.getDominator(), cur.getPostdominator()); + private static void printDomTree(PrintStream out, HIRBlock cur, String indent) { + out.printf("%s%s [dom %s, post dom %s]%n", indent, cur, cur.getDominator(), cur.getPostdominator()); HIRBlock dominated = cur.getFirstDominated(); while (dominated != null) { - printDomTree(dominated, indent + "\t"); + printDomTree(out, dominated, indent + "\t"); dominated = dominated.getDominatedSibling(); } } @Override public L enter(HIRBlock b) { - TTY.printf("%sEnter block %s for %s%n", indent, b, visitor); + out.printf("%sEnter block %s for %s%n", indent, b, visitor); indent += "\t"; return visitor.enter(b); } @@ -477,7 +481,7 @@ public L enter(HIRBlock b) { @Override public void exit(HIRBlock b, L value) { indent = indent.substring(0, indent.length() - 1); - TTY.printf("%sExit block %s with value %s for %s%n", indent, b, value, visitor); + out.printf("%sExit block %s with value %s for %s%n", indent, b, value, visitor); visitor.exit(b, value); } } From ec9fd04fafd268c243aba3d0aedd49c11887b571 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Fri, 17 Oct 2025 07:36:13 +0200 Subject: [PATCH 9/9] shared arena support: address review comments --- .../jdk/graal/compiler/core/test/LoggingCFGDecoratorTest.java | 1 - .../jdk/graal/compiler/nodes/debug/ControlFlowAnchorNode.java | 2 +- .../svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/LoggingCFGDecoratorTest.java b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/LoggingCFGDecoratorTest.java index 45f2bc780d93..2b9189caac4f 100644 --- a/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/LoggingCFGDecoratorTest.java +++ b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/LoggingCFGDecoratorTest.java @@ -44,7 +44,6 @@ public static int foo(int a) { result = 2; } GraalDirectives.controlFlowAnchor(); - ; return result; } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/debug/ControlFlowAnchorNode.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/debug/ControlFlowAnchorNode.java index d03d3d83cc24..2f7c33894082 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/debug/ControlFlowAnchorNode.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/debug/ControlFlowAnchorNode.java @@ -74,7 +74,7 @@ protected void afterClone(Node other) { @Override public Node canonical(CanonicalizerTool tool) { - if (tool.allUsagesAvailable() && numericCondition != null) { + if (numericCondition != null) { // if we have a condition that evals to 0 if (numericCondition.isConstant() && numericCondition.asJavaConstant().asLong() == 0L) { // delete this node diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java index 552bd8367fb9..a2698909266d 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nodes/foreign/MemoryArenaValidInScopeNode.java @@ -111,7 +111,7 @@ public LocationIdentity getLocationIdentity() { public void simplify(SimplifierTool tool) { if (tool.allUsagesAvailable() && graph() != null) { final ValueNode session = value; - if (session.isConstant() && session.isNullConstant()) { + if (session.isNullConstant()) { FixedNode predecessor = (FixedNode) this.predecessor(); EconomicSet scopeNodes = EconomicSet.create();