changeset 19716:1a9bfa2c3cc9

Reduce number of inserted guard proxies.
author Thomas Wuerthinger <thomas.wuerthinger@oracle.com>
date Mon, 09 Mar 2015 12:32:48 +0100
parents ff556250415e
children 70d565f6e127
files graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/ConditionalEliminationTest8.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/DominatorConditionalEliminationPhase.java
diffstat 2 files changed, 42 insertions(+), 52 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/ConditionalEliminationTest8.java	Mon Mar 09 12:03:59 2015 +0100
+++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/ConditionalEliminationTest8.java	Mon Mar 09 12:32:48 2015 +0100
@@ -47,7 +47,7 @@
 
     @Test
     public void test1() {
-        // One loop exit.
-        testProxies("test1Snippet", 1);
+        // One loop exit is skipped, because the condition dominates also the loop begin.
+        testProxies("test1Snippet", 0);
     }
 }
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/DominatorConditionalEliminationPhase.java	Mon Mar 09 12:03:59 2015 +0100
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/DominatorConditionalEliminationPhase.java	Mon Mar 09 12:32:48 2015 +0100
@@ -28,6 +28,7 @@
 import java.util.*;
 import java.util.function.*;
 
+import com.oracle.graal.compiler.common.cfg.*;
 import com.oracle.graal.compiler.common.type.*;
 import com.oracle.graal.debug.*;
 import com.oracle.graal.graph.*;
@@ -169,7 +170,7 @@
                 }
                 for (Node n : blockToNodes.apply(block)) {
                     if (n.isAlive()) {
-                        processNode(n, block, undoOperations);
+                        processNode(n, undoOperations);
                     }
                 }
                 return true;
@@ -179,26 +180,26 @@
             }
         }
 
-        private void processNode(Node node, Block block, List<Runnable> undoOperations) {
+        private void processNode(Node node, List<Runnable> undoOperations) {
             if (node instanceof AbstractBeginNode) {
                 processAbstractBegin((AbstractBeginNode) node, undoOperations);
             } else if (node instanceof FixedGuardNode) {
-                processFixedGuard((FixedGuardNode) node, block, undoOperations);
+                processFixedGuard((FixedGuardNode) node, undoOperations);
             } else if (node instanceof GuardNode) {
-                processGuard((GuardNode) node, block, undoOperations);
+                processGuard((GuardNode) node, undoOperations);
             } else if (node instanceof CheckCastNode) {
-                processCheckCast((CheckCastNode) node, block);
+                processCheckCast((CheckCastNode) node);
             } else if (node instanceof ConditionAnchorNode) {
-                processConditionAnchor((ConditionAnchorNode) node, block);
+                processConditionAnchor((ConditionAnchorNode) node);
             } else if (node instanceof IfNode) {
-                processIf((IfNode) node, block);
+                processIf((IfNode) node);
             } else {
                 return;
             }
         }
 
-        private void processCheckCast(CheckCastNode node, Block block) {
-            tryProofCondition(node, block, (guard, result) -> {
+        private void processCheckCast(CheckCastNode node) {
+            tryProofCondition(node, (guard, result) -> {
                 if (result) {
                     PiNode piNode = node.graph().unique(new PiNode(node.object(), node.stamp(), guard));
                     node.replaceAtUsages(piNode);
@@ -212,8 +213,8 @@
             });
         }
 
-        private void processIf(IfNode node, Block block) {
-            tryProofCondition(node.condition(), block, (guard, result) -> {
+        private void processIf(IfNode node) {
+            tryProofCondition(node.condition(), (guard, result) -> {
                 AbstractBeginNode survivingSuccessor = node.getSuccessor(result);
                 survivingSuccessor.replaceAtUsages(InputType.Guard, guard);
                 survivingSuccessor.replaceAtPredecessor(null);
@@ -258,15 +259,15 @@
             }
         }
 
-        private boolean rewireGuards(ValueNode guard, boolean result, Block block, BiConsumer<ValueNode, Boolean> rewireGuardFunction) {
+        private boolean rewireGuards(ValueNode guard, boolean result, BiConsumer<ValueNode, Boolean> rewireGuardFunction) {
             assert guard instanceof GuardingNode;
             metricStampsFound.increment();
-            ValueNode proxiedGuard = proxyGuard(guard, block);
+            ValueNode proxiedGuard = proxyGuard(guard);
             rewireGuardFunction.accept(proxiedGuard, result);
             return true;
         }
 
-        private ValueNode proxyGuard(ValueNode guard, Block block) {
+        private ValueNode proxyGuard(ValueNode guard) {
             ValueNode proxiedGuard = guard;
             if (!this.loopExits.isEmpty()) {
                 while (proxiedGuard instanceof GuardProxyNode) {
@@ -274,33 +275,21 @@
                 }
                 Block guardBlock = nodeToBlock.apply(proxiedGuard);
                 assert guardBlock != null;
-                Block curBlock = block;
-                if (guardBlock != curBlock) {
-                    int loopExitIndex = loopExits.size();
-                    do {
-                        LoopExitNode loopExitNode = loopExits.get(loopExitIndex - 1);
-                        Block loopExitBlock = nodeToBlock.apply(loopExitNode);
-                        assert loopExitBlock != null;
-                        if (loopExitBlock == curBlock) {
-                            assert curBlock != guardBlock;
-                            loopExitIndex--;
-                            if (loopExitIndex <= 0) {
-                                break;
-                            }
+                for (int i = 0; i < loopExits.size(); ++i) {
+                    LoopExitNode loopExitNode = loopExits.get(i);
+                    Block loopExitBlock = nodeToBlock.apply(loopExitNode);
+                    if (guardBlock != loopExitBlock && AbstractControlFlowGraph.dominates(guardBlock, loopExitBlock)) {
+                        Block loopBeginBlock = nodeToBlock.apply(loopExitNode.loopBegin());
+                        if (!AbstractControlFlowGraph.dominates(guardBlock, loopBeginBlock) || guardBlock == loopBeginBlock) {
+                            proxiedGuard = proxiedGuard.graph().unique(new GuardProxyNode((GuardingNode) proxiedGuard, loopExitNode));
                         }
-                        curBlock = curBlock.getDominator();
-                    } while (guardBlock != curBlock);
-
-                    for (int i = loopExitIndex; i < loopExits.size(); ++i) {
-                        LoopExitNode loopExitNode = loopExits.get(i);
-                        proxiedGuard = proxiedGuard.graph().unique(new GuardProxyNode((GuardingNode) proxiedGuard, loopExitNode));
                     }
                 }
             }
             return proxiedGuard;
         }
 
-        private boolean tryProofCondition(Node node, Block block, BiConsumer<ValueNode, Boolean> rewireGuardFunction) {
+        private boolean tryProofCondition(Node node, BiConsumer<ValueNode, Boolean> rewireGuardFunction) {
             if (node instanceof UnaryOpLogicNode) {
                 UnaryOpLogicNode unaryLogicNode = (UnaryOpLogicNode) node;
                 ValueNode value = unaryLogicNode.getValue();
@@ -308,16 +297,16 @@
                     Stamp stamp = infoElement.getStamp();
                     Boolean result = unaryLogicNode.tryFold(stamp);
                     if (result != null) {
-                        return rewireGuards(infoElement.getGuard(), result, block, rewireGuardFunction);
+                        return rewireGuards(infoElement.getGuard(), result, rewireGuardFunction);
                     }
                 }
             } else if (node instanceof BinaryOpLogicNode) {
                 BinaryOpLogicNode binaryOpLogicNode = (BinaryOpLogicNode) node;
                 for (InfoElement infoElement : getInfoElements(binaryOpLogicNode)) {
                     if (infoElement.getStamp().equals(StampFactory.contradiction())) {
-                        return rewireGuards(infoElement.getGuard(), false, block, rewireGuardFunction);
+                        return rewireGuards(infoElement.getGuard(), false, rewireGuardFunction);
                     } else if (infoElement.getStamp().equals(StampFactory.tautology())) {
-                        return rewireGuards(infoElement.getGuard(), true, block, rewireGuardFunction);
+                        return rewireGuards(infoElement.getGuard(), true, rewireGuardFunction);
                     }
                 }
 
@@ -326,14 +315,14 @@
                 for (InfoElement infoElement : getInfoElements(x)) {
                     Boolean result = binaryOpLogicNode.tryFold(infoElement.getStamp(), y.stamp());
                     if (result != null) {
-                        return rewireGuards(infoElement.getGuard(), result, block, rewireGuardFunction);
+                        return rewireGuards(infoElement.getGuard(), result, rewireGuardFunction);
                     }
                 }
 
                 for (InfoElement infoElement : getInfoElements(y)) {
                     Boolean result = binaryOpLogicNode.tryFold(x.stamp(), infoElement.getStamp());
                     if (result != null) {
-                        return rewireGuards(infoElement.getGuard(), result, block, rewireGuardFunction);
+                        return rewireGuards(infoElement.getGuard(), result, rewireGuardFunction);
                     }
                 }
             } else if (node instanceof CheckCastNode) {
@@ -341,19 +330,19 @@
                 for (InfoElement infoElement : getInfoElements(checkCastNode.object())) {
                     Boolean result = checkCastNode.tryFold(infoElement.getStamp());
                     if (result != null) {
-                        return rewireGuards(infoElement.getGuard(), result, block, rewireGuardFunction);
+                        return rewireGuards(infoElement.getGuard(), result, rewireGuardFunction);
                     }
                 }
             } else if (node instanceof ShortCircuitOrNode) {
                 final ShortCircuitOrNode shortCircuitOrNode = (ShortCircuitOrNode) node;
                 if (this.loopExits.isEmpty()) {
-                    tryProofCondition(shortCircuitOrNode.getX(), block, (guard, result) -> {
+                    tryProofCondition(shortCircuitOrNode.getX(), (guard, result) -> {
                         if (result == !shortCircuitOrNode.isXNegated()) {
-                            rewireGuards(guard, result, block, rewireGuardFunction);
+                            rewireGuards(guard, result, rewireGuardFunction);
                         } else {
-                            tryProofCondition(shortCircuitOrNode.getY(), block, (innerGuard, innerResult) -> {
+                            tryProofCondition(shortCircuitOrNode.getY(), (innerGuard, innerResult) -> {
                                 if (innerGuard == guard) {
-                                    rewireGuards(guard, shortCircuitOrNode.isYNegated() ? !innerResult : innerResult, block, rewireGuardFunction);
+                                    rewireGuards(guard, shortCircuitOrNode.isYNegated() ? !innerResult : innerResult, rewireGuardFunction);
                                 }
                             });
                         }
@@ -379,8 +368,8 @@
             }
         }
 
-        private void processConditionAnchor(ConditionAnchorNode node, Block block) {
-            tryProofCondition(node.condition(), block, (guard, result) -> {
+        private void processConditionAnchor(ConditionAnchorNode node) {
+            tryProofCondition(node.condition(), (guard, result) -> {
                 if (result == node.isNegated()) {
                     node.replaceAtUsages(guard);
                     GraphUtil.unlinkFixedNode(node);
@@ -393,12 +382,13 @@
             });
         }
 
-        private void processGuard(GuardNode node, Block block, List<Runnable> undoOperations) {
-            if (!tryProofCondition(node.condition(), block, (guard, result) -> {
+        private void processGuard(GuardNode node, List<Runnable> undoOperations) {
+            if (!tryProofCondition(node.condition(), (guard, result) -> {
                 if (result != node.isNegated()) {
                     node.replaceAndDelete(guard);
                 } else {
                     DeoptimizeNode deopt = node.graph().add(new DeoptimizeNode(node.action(), node.reason()));
+                    Block block = nodeToBlock.apply(node);
                     FixedNode next = block.getBeginNode().next();
                     block.getBeginNode().setNext(deopt);
                     GraphUtil.killCFG(next);
@@ -408,8 +398,8 @@
             }
         }
 
-        private void processFixedGuard(FixedGuardNode node, Block block, List<Runnable> undoOperations) {
-            if (!tryProofCondition(node.condition(), block, (guard, result) -> {
+        private void processFixedGuard(FixedGuardNode node, List<Runnable> undoOperations) {
+            if (!tryProofCondition(node.condition(), (guard, result) -> {
                 if (result != node.isNegated()) {
                     node.replaceAtUsages(guard);
                     GraphUtil.unlinkFixedNode(node);