Mercurial > hg > truffle

changeset 21928:130e06e34659

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
TypeSwitchNode should be more careful about deleting successors
author Tom Rodriguez <tom.rodriguez@oracle.com>
date Thu, 11 Jun 2015 13:03:54 -0700
parents 6a93800d10f1
children ae5bee2d61b2
files graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/IntegerSwitchNode.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/SwitchNode.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/TypeSwitchNode.java
diffstat 3 files changed, 29 insertions(+), 22 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/IntegerSwitchNode.java	Thu Jun 11 12:15:19 2015 -0700
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/IntegerSwitchNode.java	Thu Jun 11 13:03:54 2015 -0700
@@ -99,12 +99,16 @@
     }
 
     public AbstractBeginNode successorAtKey(int key) {
+        return blockSuccessor(successorIndexAtKey(key));
+    }
+
+    public int successorIndexAtKey(int key) {
         for (int i = 0; i < keyCount(); i++) {
             if (keys[i] == key) {
-                return blockSuccessor(keySuccessorIndex(i));
+                return keySuccessorIndex(i);
             }
         }
-        return blockSuccessor(keySuccessorIndex(keyCount()));
+        return keySuccessorIndex(keyCount());
     }
 
     @Override
@@ -113,18 +117,7 @@
             tool.addToWorkList(defaultSuccessor());
             graph().removeSplitPropagate(this, defaultSuccessor());
         } else if (value() instanceof ConstantNode) {
-            int constant = value().asJavaConstant().asInt();
-            AbstractBeginNode survivingSuccessor = successorAtKey(constant);
-            for (Node successor : successors()) {
-                if (successor != survivingSuccessor) {
-                    tool.deleteBranch(successor);
-                    // deleteBranch can change the successors so reload it
-                    survivingSuccessor = successorAtKey(constant);
-                }
-            }
-            tool.addToWorkList(survivingSuccessor);
-            graph().removeSplit(this, survivingSuccessor);
-
+            killOtherSuccessors(tool, successorIndexAtKey(value().asJavaConstant().asInt()));
         } else if (value().stamp() instanceof IntegerStamp) {
             IntegerStamp integerStamp = (IntegerStamp) value().stamp();
             if (!integerStamp.isUnrestricted()) {
@@ -179,7 +172,7 @@
                     }
 
                     AbstractBeginNode[] successorsArray = newSuccessors.toArray(new AbstractBeginNode[newSuccessors.size()]);
-                    IntegerSwitchNode newSwitch = graph().add(new IntegerSwitchNode(value(), successorsArray, newKeys, newKeyProbabilities, newKeySuccessors));
+                    SwitchNode newSwitch = graph().add(new IntegerSwitchNode(value(), successorsArray, newKeys, newKeyProbabilities, newKeySuccessors));
                     ((FixedWithNextNode) predecessor()).setNext(newSwitch);
                     GraphUtil.killWithUnusedFloatingInputs(this);
                 }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/SwitchNode.java	Thu Jun 11 12:15:19 2015 -0700
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/SwitchNode.java	Thu Jun 11 13:03:54 2015 -0700
@@ -26,6 +26,7 @@
 
 import com.oracle.graal.compiler.common.type.*;
 import com.oracle.graal.graph.*;
+import com.oracle.graal.graph.spi.*;
 import com.oracle.graal.nodeinfo.*;
 import com.oracle.graal.nodes.*;
 import com.oracle.jvmci.common.*;
@@ -164,4 +165,23 @@
     public AbstractBeginNode getPrimarySuccessor() {
         return this.defaultSuccessor();
     }
+
+    /**
+     * Delete all other successors except for the one reached by {@code survivingEdge}. Deleting a
+     * branch can change the successors and the same successor might appear multiple times in the
+     * {@link #successors} list, so use node identity for the comparision and don't cache the
+     * surviving AbstractBeginNode.
+     *
+     * @param tool
+     * @param survivingEdge index of the edge in the {@link SwitchNode#successors} list
+     */
+    protected void killOtherSuccessors(SimplifierTool tool, int survivingEdge) {
+        for (Node successor : successors()) {
+            if (successor != blockSuccessor(survivingEdge)) {
+                tool.deleteBranch(successor);
+            }
+        }
+        tool.addToWorkList(blockSuccessor(survivingEdge));
+        graph().removeSplit(this, blockSuccessor(survivingEdge));
+    }
 }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/TypeSwitchNode.java	Thu Jun 11 12:15:19 2015 -0700
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/java/TypeSwitchNode.java	Thu Jun 11 13:03:54 2015 -0700
@@ -131,13 +131,7 @@
                     survivingEdge = keySuccessorIndex(i);
                 }
             }
-            for (int i = 0; i < blockSuccessorCount(); i++) {
-                if (i != survivingEdge) {
-                    tool.deleteBranch(blockSuccessor(i));
-                }
-            }
-            tool.addToWorkList(blockSuccessor(survivingEdge));
-            graph().removeSplit(this, blockSuccessor(survivingEdge));
+            killOtherSuccessors(tool, survivingEdge);
         }
         if (value() instanceof LoadHubNode && ((LoadHubNode) value()).getValue().stamp() instanceof ObjectStamp) {
             ObjectStamp objectStamp = (ObjectStamp) ((LoadHubNode) value()).getValue().stamp();