Treat phi nodes the same as fixed nodes in LoopFragment detection.

fengno · Feb 23, 2018 · 37436c5 · 37436c5
1 parent 31c3b3c
commit 37436c5
Showing 1 changed file with 20 additions and 16 deletions.
diff --git a/compiler/src/org.graalvm.compiler.loop/src/org/graalvm/compiler/loop/LoopFragment.java b/compiler/src/org.graalvm.compiler.loop/src/org/graalvm/compiler/loop/LoopFragment.java
@@ -35,6 +35,7 @@
 import org.graalvm.compiler.graph.NodeBitMap;
 import org.graalvm.compiler.graph.iterators.NodeIterable;
 import org.graalvm.compiler.nodes.AbstractBeginNode;
+import org.graalvm.compiler.nodes.AbstractMergeNode;
 import org.graalvm.compiler.nodes.EndNode;
 import org.graalvm.compiler.nodes.FixedNode;
 import org.graalvm.compiler.nodes.FrameState;
@@ -208,6 +209,12 @@ protected static void computeNodes(NodeBitMap nodes, Graph graph, Iterable<Abstr
                     NodeWithState withState = (NodeWithState) n;
                     withState.states().forEach(state -> state.applyToVirtual(node -> nodes.mark(node)));
                 }
+                if (n instanceof AbstractMergeNode) {
+                    // if a merge is in the loop, all of its phis are also in the loop
+                    for (PhiNode phi : ((AbstractMergeNode) n).phis()) {
+                        nodes.mark(phi);
+                    }
+                }
                 nodes.mark(n);
             }
         }
@@ -246,6 +253,17 @@ protected static void computeNodes(NodeBitMap nodes, Graph graph, Iterable<Abstr
                 if (n instanceof MonitorEnterNode) {
                     markFloating(worklist, ((MonitorEnterNode) n).getMonitorId(), nodes, nonLoopNodes);
                 }
+                if (n instanceof AbstractMergeNode) {
+                    /*
+                     * Since we already marked all phi nodes as being in the loop to break cycles,
+                     * we also have to iterate over their usages here.
+                     */
+                    for (PhiNode phi : ((AbstractMergeNode) n).phis()) {
+                        for (Node usage : phi.usages()) {
+                            markFloating(worklist, usage, nodes, nonLoopNodes);
+                        }
+                    }
+                }
                 for (Node usage : n.usages()) {
                     markFloating(worklist, usage, nodes, nonLoopNodes);
                 }
@@ -286,24 +304,10 @@ static TriState isLoopNode(Node n, NodeBitMap loopNodes, NodeBitMap nonLoopNodes
         if (nonLoopNodes.isMarked(n)) {
             return TriState.FALSE;
         }
-        if (n instanceof FixedNode) {
+        if (n instanceof FixedNode || n instanceof PhiNode) {
+            // phi nodes are treated the same as fixed nodes in this algorithm to break cycles
             return TriState.FALSE;
         }
-        boolean mark = false;
-        if (n instanceof PhiNode) {
-            PhiNode phi = (PhiNode) n;
-            mark = loopNodes.isMarked(phi.merge());
-            if (mark) {
-                /*
-                 * This Phi is a loop node but the inputs might not be so they must be processed by
-                 * the caller.
-                 */
-                loopNodes.mark(n);
-            } else {
-                nonLoopNodes.mark(n);
-                return TriState.FALSE;
-            }
-        }
         return TriState.UNKNOWN;
     }