loop utility: unify canUseWithoutProxy

oracle · Jan 25, 2024 · 2f1f657 · 2f1f657
1 parent ec7c3f0
commit 2f1f657
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 18 deletions.
diff --git a/...ompiler/src/jdk/graal/compiler/phases/common/DominatorBasedGlobalValueNumberingPhase.java b/...ompiler/src/jdk/graal/compiler/phases/common/DominatorBasedGlobalValueNumberingPhase.java
@@ -28,6 +28,7 @@
 import java.util.Arrays;
 import java.util.Optional;
 
+import jdk.graal.compiler.phases.common.util.LoopUtility;
 import jdk.graal.compiler.phases.util.GraphOrder;
 import org.graalvm.collections.EconomicMap;
 import jdk.graal.compiler.core.common.GraalOptions;
@@ -612,24 +613,9 @@ public void substitute(Node n, ControlFlowGraph cfg, NodeBitMap licmNodes, LoopE
                     }
                 }
 
-                HIRBlock useBlock = cfg.blockFor(n);
-                Loop<HIRBlock> defLoop = defBlock.getLoop();
-                Loop<HIRBlock> useLoop = useBlock.getLoop();
-
-                if (defLoop != null) {
-                    // the def is inside a loop, either a parent or a disjunct loop
-                    if (useLoop != null) {
-                        // we are only safe without proxies if we are included in the def loop,
-                        // i.e., the def loop is a parent loop
-                        if (!useLoop.isAncestorOrSelf(defLoop)) {
-                            earlyGVNAbort.increment(graph.getDebug());
-                            return;
-                        }
-                    } else {
-                        // the use is not in a loop but the def is, needs proxies, fail
-                        earlyGVNAbort.increment(graph.getDebug());
-                        return;
-                    }
+                if (!LoopUtility.canUseWithoutProxy(cfg, edgeDataEqual, n)) {
+                    earlyGVNAbort.increment(graph.getDebug());
+                    return;
                 }
 
                 graph.getDebug().log(DebugContext.VERY_DETAILED_LEVEL, "Early GVN: replacing %s with %s", n, edgeDataEqual);

diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/util/LoopUtility.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/util/LoopUtility.java
@@ -26,10 +26,14 @@
 
 import java.util.EnumSet;
 
+import jdk.graal.compiler.core.common.cfg.Loop;
 import jdk.graal.compiler.core.common.type.IntegerStamp;
 import jdk.graal.compiler.core.common.type.Stamp;
 import jdk.graal.compiler.graph.Graph.NodeEvent;
 import jdk.graal.compiler.graph.Graph.NodeEventScope;
+import jdk.graal.compiler.graph.Node;
+import jdk.graal.compiler.nodes.FixedNode;
+import jdk.graal.compiler.nodes.GraphState.StageFlag;
 import jdk.graal.compiler.nodes.LoopExitNode;
 import jdk.graal.compiler.nodes.NodeView;
 import jdk.graal.compiler.nodes.ProxyNode;
@@ -39,6 +43,8 @@
 import jdk.graal.compiler.nodes.calc.AddNode;
 import jdk.graal.compiler.nodes.calc.IntegerConvertNode;
 import jdk.graal.compiler.nodes.calc.MulNode;
+import jdk.graal.compiler.nodes.cfg.ControlFlowGraph;
+import jdk.graal.compiler.nodes.cfg.HIRBlock;
 import jdk.graal.compiler.nodes.loop.BasicInductionVariable;
 import jdk.graal.compiler.nodes.loop.InductionVariable;
 import jdk.graal.compiler.nodes.loop.LoopEx;
@@ -48,6 +54,47 @@
 
 public class LoopUtility {
 
+    /**
+     * Determine if the def can use node {@code use} without the need for value proxies. This means
+     * there is no loop exit between the schedule point of def and use that would require a
+     * {@link ProxyNode}.
+     */
+    public static boolean canUseWithoutProxy(ControlFlowGraph cfg, Node def, Node use) {
+        if (def.graph() instanceof StructuredGraph g && g.isAfterStage(StageFlag.VALUE_PROXY_REMOVAL)) {
+            return true;
+        }
+        if (!isFixedNode(def) || !isFixedNode(use)) {
+            /*
+             * If def or use are not fixed nodes we cannot determine the schedule point for them.
+             * Without the schedule point we cannot find their basic block in the control flow
+             * graph. If we would schedule the graph we could answer the question for floating nodes
+             * as well but this is too much overhead. Thus, for floating nodes we give up and assume
+             * a proxy is necessary.
+             */
+            return false;
+        }
+        HIRBlock useBlock = cfg.blockFor(use);
+        HIRBlock defBlock = cfg.blockFor(def);
+        Loop<HIRBlock> defLoop = defBlock.getLoop();
+        Loop<HIRBlock> useLoop = useBlock.getLoop();
+        if (defLoop != null) {
+            // the def is inside a loop, either a parent or a disjunct loop
+            if (useLoop != null) {
+                // we are only safe without proxies if we are included in the def loop,
+                // i.e., the def loop is a parent loop
+                return useLoop.isAncestorOrSelf(defLoop);
+            } else {
+                // the use is not in a loop but the def is, needs proxies, fail
+                return false;
+            }
+        }
+        return true;
+    }
+
+    private static boolean isFixedNode(Node n) {
+        return n instanceof FixedNode;
+    }
+
     public static boolean isNumericInteger(ValueNode v) {
         Stamp s = v.stamp(NodeView.DEFAULT);
         return s instanceof IntegerStamp;