changeset 12401:85dcc7f59c34

Truffle-DSL: fixed incorrect else guard connections for executeAndSpecialize.
author Christian Humer <christian.humer@gmail.com>
date Mon, 14 Oct 2013 15:44:18 +0200
parents 28e7396dca1d
children bfcae72b61a0
files graal/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/SpecializationGroupingTest.java graal/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/TestHelper.java graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/node/NodeCodeGenerator.java graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/node/SpecializationGroup.java
diffstat 4 files changed, 54 insertions(+), 33 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/SpecializationGroupingTest.java	Mon Oct 14 14:32:00 2013 +0200
+++ b/graal/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/SpecializationGroupingTest.java	Mon Oct 14 15:44:18 2013 +0200
@@ -37,7 +37,8 @@
 
 /**
  * Tests execution counts of guards. While we do not make guarantees for guard invocation except for
- * their execution order our implementation reduces the calls to guards as much as possible.
+ * their execution order our implementation reduces the calls to guards as much as possible for the
+ * generic case.
  */
 public class SpecializationGroupingTest {
 
@@ -47,7 +48,7 @@
         MockAssumption a2 = new MockAssumption(false);
         MockAssumption a3 = new MockAssumption(true);
 
-        TestRootNode<TestGrouping> root = TestHelper.createRoot(TestGroupingFactory.getInstance(), a1, a2, a3);
+        TestRootNode<TestGrouping> root = TestHelper.createGenericRoot(TestGroupingFactory.getInstance(), a1, a2, a3);
 
         SimpleTypes.intCast = 0;
         SimpleTypes.intCheck = 0;
@@ -71,16 +72,16 @@
 
         Assert.assertEquals(42, TestHelper.executeWith(root, 21, 21));
         Assert.assertEquals(2, TestGrouping.true1);
-        Assert.assertEquals(1, TestGrouping.false1);
+        Assert.assertEquals(2, TestGrouping.false1);
         Assert.assertEquals(2, TestGrouping.true2);
         Assert.assertEquals(2, TestGrouping.false2);
         Assert.assertEquals(2, TestGrouping.true3);
 
         Assert.assertEquals(2, a1.checked);
-        Assert.assertEquals(1, a2.checked);
+        Assert.assertEquals(2, a2.checked);
         Assert.assertEquals(2, a3.checked);
-        Assert.assertEquals(2, SimpleTypes.intCheck);
-        Assert.assertEquals(2, SimpleTypes.intCast);
+        Assert.assertEquals(4, SimpleTypes.intCheck);
+        Assert.assertEquals(4, SimpleTypes.intCast);
 
     }
 
--- a/graal/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/TestHelper.java	Mon Oct 14 14:32:00 2013 +0200
+++ b/graal/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/TestHelper.java	Mon Oct 14 15:44:18 2013 +0200
@@ -61,10 +61,18 @@
         return factory.createNode(argumentList.toArray(new Object[argumentList.size()]));
     }
 
+    static <E extends ValueNode> E createGenericNode(NodeFactory<E> factory, Object... constants) {
+        return factory.createNodeGeneric(createNode(factory, constants));
+    }
+
     static <E extends ValueNode> TestRootNode<E> createRoot(NodeFactory<E> factory, Object... constants) {
         return new TestRootNode<>(createNode(factory, constants));
     }
 
+    static <E extends ValueNode> TestRootNode<E> createGenericRoot(NodeFactory<E> factory, Object... constants) {
+        return new TestRootNode<>(createGenericNode(factory, constants));
+    }
+
     static CallTarget createCallTarget(ValueNode node) {
         return createCallTarget(new TestRootNode<>(node));
     }
--- a/graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/node/NodeCodeGenerator.java	Mon Oct 14 14:32:00 2013 +0200
+++ b/graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/node/NodeCodeGenerator.java	Mon Oct 14 15:44:18 2013 +0200
@@ -1391,7 +1391,7 @@
                 return true;
             }
             SpecializationGroup previous = group.getPreviousGroup();
-            if (previous == null || previous.getElseConnectableGuards().isEmpty()) {
+            if (previous == null || previous.findElseConnectableGuards(checkMinimumState).isEmpty()) {
                 return true;
             }
 
@@ -1417,30 +1417,11 @@
             String guardsAnd = "";
             String guardsCastAnd = "";
 
-            List<GuardData> elseGuards = group.getElseConnectableGuards();
-
             boolean minimumState = checkMinimumState;
             if (minimumState) {
-                int groupMaxIndex = group.getMaxSpecializationIndex();
-
-                int genericIndex = node.getSpecializations().indexOf(node.getGenericSpecialization());
-                if (groupMaxIndex >= genericIndex) {
-                    // no minimum state check for an generic index
-                    minimumState = false;
-                }
-
-                if (minimumState) {
-                    // no minimum state check if alread checked by parent group
-                    int parentMaxIndex = -1;
-                    if (group.getParent() != null) {
-                        parentMaxIndex = group.getParent().getMaxSpecializationIndex();
-                    }
-                    if (groupMaxIndex == parentMaxIndex) {
-                        minimumState = false;
-                    }
-                }
-
-                if (minimumState) {
+                int groupMaxIndex = group.getUncheckedSpecializationIndex();
+
+                if (groupMaxIndex > -1) {
                     guardsBuilder.string(guardsAnd);
                     guardsBuilder.string("minimumState < " + groupMaxIndex);
                     guardsAnd = " && ";
@@ -1489,6 +1470,7 @@
                     castBuilder.tree(cast);
                 }
             }
+            List<GuardData> elseGuards = group.findElseConnectableGuards(checkMinimumState);
 
             for (GuardData guard : group.getGuards()) {
                 if (elseGuards.contains(guard)) {
--- a/graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/node/SpecializationGroup.java	Mon Oct 14 14:32:00 2013 +0200
+++ b/graal/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/node/SpecializationGroup.java	Mon Oct 14 15:44:18 2013 +0200
@@ -90,7 +90,14 @@
         return null;
     }
 
-    public List<GuardData> getElseConnectableGuards() {
+    public List<GuardData> findElseConnectableGuards(boolean minimumStateCheck) {
+        if (minimumStateCheck) {
+            /*
+             * TODO investigate further if we really cannot else connect guards if minimum state is
+             * required
+             */
+            return Collections.emptyList();
+        }
         if (!getTypeGuards().isEmpty() || !getAssumptions().isEmpty()) {
             return Collections.emptyList();
         }
@@ -101,7 +108,7 @@
 
         List<GuardData> elseConnectableGuards = new ArrayList<>();
         int guardIndex = 0;
-        while (guardIndex < getGuards().size() && findNegatedGuardInPrevious(getGuards().get(guardIndex)) != null) {
+        while (guardIndex < getGuards().size() && findNegatedGuardInPrevious(getGuards().get(guardIndex), minimumStateCheck) != null) {
             elseConnectableGuards.add(getGuards().get(guardIndex));
             guardIndex++;
         }
@@ -109,17 +116,18 @@
         return elseConnectableGuards;
     }
 
-    private GuardData findNegatedGuardInPrevious(GuardData guard) {
+    private GuardData findNegatedGuardInPrevious(GuardData guard, boolean minimumStateCheck) {
         SpecializationGroup previous = this.getPreviousGroup();
         if (previous == null) {
             return null;
         }
-        List<GuardData> elseConnectedGuards = previous.getElseConnectableGuards();
+        List<GuardData> elseConnectedGuards = previous.findElseConnectableGuards(minimumStateCheck);
 
         if (previous == null || previous.getGuards().size() != elseConnectedGuards.size() + 1) {
             return null;
         }
 
+        /* Guard is else branch can be connected in previous specialization. */
         if (elseConnectedGuards.contains(guard)) {
             return guard;
         }
@@ -341,6 +349,28 @@
         return parent.children.get(index - 1);
     }
 
+    public int getUncheckedSpecializationIndex() {
+        int groupMaxIndex = getMaxSpecializationIndex();
+
+        int genericIndex = node.getSpecializations().indexOf(node.getGenericSpecialization());
+        if (groupMaxIndex >= genericIndex) {
+            // no minimum state check for an generic index
+            groupMaxIndex = -1;
+        }
+
+        if (groupMaxIndex > -1) {
+            // no minimum state check if already checked by parent group
+            int parentMaxIndex = -1;
+            if (getParent() != null) {
+                parentMaxIndex = getParent().getMaxSpecializationIndex();
+            }
+            if (groupMaxIndex == parentMaxIndex) {
+                groupMaxIndex = -1;
+            }
+        }
+        return groupMaxIndex;
+    }
+
     public int getMaxSpecializationIndex() {
         if (specialization != null) {
             return specialization.getNode().getSpecializations().indexOf(specialization);