changeset 22885:dc1551f0833e

Fold post dominating bit tests into earlier guards
author Tom Rodriguez <tom.rodriguez@oracle.com>
date Sat, 24 Oct 2015 15:29:31 -0700
parents 2fe4e3511d97
children 86dacea931a2
files graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/ConditionalEliminationTest11.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/DominatorConditionalEliminationPhase.java
diffstat 2 files changed, 254 insertions(+), 0 deletions(-) [+]
line wrap: on
line diff
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/ConditionalEliminationTest11.java	Sat Oct 24 15:29:31 2015 -0700
@@ -0,0 +1,167 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.oracle.graal.compiler.test;
+
+import org.junit.Ignore;
+import org.junit.Test;
+
+import com.oracle.graal.api.directives.GraalDirectives;
+
+/**
+ * Collection of tests for
+ * {@link com.oracle.graal.phases.common.DominatorConditionalEliminationPhase} including those that
+ * triggered bugs in this phase.
+ */
+public class ConditionalEliminationTest11 extends ConditionalEliminationTestBase {
+
+    @SuppressWarnings("all")
+    public static int referenceSnippet(int a) {
+        if ((a & 15) != 15) {
+            GraalDirectives.deoptimize();
+        }
+        return 0;
+    }
+
+    @Test
+    public void test1() {
+        testConditionalElimination("test1Snippet", "referenceSnippet");
+    }
+
+    @SuppressWarnings("all")
+    public static int test1Snippet(int a) {
+        if ((a & 8) != 8) {
+            GraalDirectives.deoptimize();
+        }
+        if ((a & 15) != 15) {
+            GraalDirectives.deoptimize();
+        }
+        return 0;
+    }
+
+    @SuppressWarnings("all")
+    public static int test2Snippet(int a) {
+        if ((a & 8) == 0) {
+            GraalDirectives.deoptimize();
+        }
+        if ((a & 15) != 15) {
+            GraalDirectives.deoptimize();
+        }
+        return 0;
+    }
+
+    @Test
+    public void test2() {
+        testConditionalElimination("test2Snippet", "referenceSnippet");
+    }
+
+    @SuppressWarnings("all")
+    public static int test3Snippet(int a) {
+        if ((a & 15) != 15) {
+            GraalDirectives.deoptimize();
+        }
+        if ((a & 8) != 8) {
+            GraalDirectives.deoptimize();
+        }
+        return 0;
+    }
+
+    @Test
+    public void test3() {
+        // Test forward elimination of bitwise tests
+        testConditionalElimination("test3Snippet", "referenceSnippet");
+    }
+
+    @SuppressWarnings("all")
+    public static int test4Snippet(int a) {
+        if ((a & 15) != 15) {
+            GraalDirectives.deoptimize();
+        }
+        if ((a & 8) == 0) {
+            GraalDirectives.deoptimize();
+        }
+        return 0;
+    }
+
+    @Test
+    public void test4() {
+        // Test forward elimination of bitwise tests
+        testConditionalElimination("test4Snippet", "referenceSnippet");
+    }
+
+    @SuppressWarnings("all")
+    public static int reference5Snippet(int a) {
+        if ((a & 15) == 15) {
+            GraalDirectives.deoptimize();
+        }
+        if (a > 0) {
+            a |= 32;
+        }
+        return a;
+    }
+
+    @SuppressWarnings("all")
+    public static int test5Snippet(int a) {
+        if ((a & 8) != 8) {
+            GraalDirectives.deoptimize();
+        }
+        if (a > 0) {
+            a |= 32;
+        }
+        if ((a & 15) == 15) {
+            GraalDirectives.deoptimize();
+        }
+        return a;
+    }
+
+    @Ignore
+    @Test
+    public void test5() {
+        testConditionalElimination("test5Snippet", "test5Snippet");
+    }
+
+    @SuppressWarnings("all")
+    public static int test6Snippet(int a) {
+        if ((a & 8) != 0) {
+            GraalDirectives.deoptimize();
+        }
+        if ((a & 15) != 15) {
+            GraalDirectives.deoptimize();
+        }
+        return 0;
+    }
+
+    @SuppressWarnings("all")
+    public static int reference6Snippet(int a) {
+        if ((a & 8) != 0) {
+            GraalDirectives.deoptimize();
+        }
+        GraalDirectives.deoptimize();
+        return 0;
+    }
+
+    @Test
+    public void test6() {
+        // Shouldn't be able to optimize this
+        testConditionalElimination("test6Snippet", "reference6Snippet");
+    }
+}
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/DominatorConditionalEliminationPhase.java	Sat Oct 24 15:27:58 2015 -0700
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/DominatorConditionalEliminationPhase.java	Sat Oct 24 15:29:31 2015 -0700
@@ -38,6 +38,11 @@
 
 import com.oracle.graal.compiler.common.cfg.AbstractControlFlowGraph;
 import com.oracle.graal.compiler.common.cfg.BlockMap;
+import com.oracle.graal.compiler.common.type.ArithmeticOpTable;
+import com.oracle.graal.compiler.common.type.ArithmeticOpTable.BinaryOp;
+import com.oracle.graal.compiler.common.type.ArithmeticOpTable.BinaryOp.And;
+import com.oracle.graal.compiler.common.type.ArithmeticOpTable.BinaryOp.Or;
+import com.oracle.graal.compiler.common.type.IntegerStamp;
 import com.oracle.graal.compiler.common.type.Stamp;
 import com.oracle.graal.compiler.common.type.StampFactory;
 import com.oracle.graal.debug.Debug;
@@ -62,7 +67,9 @@
 import com.oracle.graal.nodes.StructuredGraph;
 import com.oracle.graal.nodes.UnaryOpLogicNode;
 import com.oracle.graal.nodes.ValueNode;
+import com.oracle.graal.nodes.calc.AndNode;
 import com.oracle.graal.nodes.calc.BinaryArithmeticNode;
+import com.oracle.graal.nodes.calc.IntegerEqualsNode;
 import com.oracle.graal.nodes.cfg.Block;
 import com.oracle.graal.nodes.cfg.ControlFlowGraph;
 import com.oracle.graal.nodes.extended.GuardingNode;
@@ -71,6 +78,7 @@
 import com.oracle.graal.nodes.extended.ValueAnchorNode;
 import com.oracle.graal.nodes.java.CheckCastNode;
 import com.oracle.graal.nodes.java.TypeSwitchNode;
+import com.oracle.graal.nodes.spi.NodeWithState;
 import com.oracle.graal.nodes.util.GraphUtil;
 import com.oracle.graal.phases.Phase;
 import com.oracle.graal.phases.common.LoweringPhase.Frame;
@@ -166,6 +174,18 @@
         instance.processBlock(startBlock);
     }
 
+    static class PendingTest {
+        private final LogicNode condition;
+        private final boolean negated;
+        private final ValueNode guard;
+
+        public PendingTest(LogicNode condition, boolean negated, ValueNode guard) {
+            this.condition = condition;
+            this.negated = negated;
+            this.guard = guard;
+        }
+    }
+
     private static class Instance {
 
         private NodeMap<Info> map;
@@ -173,11 +193,17 @@
         private final Function<Block, Iterable<? extends Node>> blockToNodes;
         private final Function<Node, Block> nodeToBlock;
 
+        /**
+         * Tests which may be eliminated because post dominating tests to prove a broader condition.
+         */
+        private Deque<PendingTest> pendingTests;
+
         public Instance(StructuredGraph graph, Function<Block, Iterable<? extends Node>> blockToNodes, Function<Node, Block> nodeToBlock) {
             map = graph.createNodeMap();
             loopExits = new ArrayDeque<>();
             this.blockToNodes = blockToNodes;
             this.nodeToBlock = nodeToBlock;
+            pendingTests = new ArrayDeque<>();
         }
 
         public void processBlock(Block startBlock) {
@@ -232,6 +258,8 @@
                     loopExits = oldLoopExits;
                 });
             }
+            // For now conservatively collect guards only within the same block.
+            pendingTests.clear();
             for (Node n : blockToNodes.apply(block)) {
                 if (n.isAlive()) {
                     processNode(n, undoOperations);
@@ -240,6 +268,9 @@
         }
 
         private void processNode(Node node, List<Runnable> undoOperations) {
+            if (node instanceof NodeWithState && !(node instanceof GuardingNode)) {
+                pendingTests.clear();
+            }
             if (node instanceof AbstractBeginNode) {
                 processAbstractBegin((AbstractBeginNode) node, undoOperations);
             } else if (node instanceof FixedGuardNode) {
@@ -316,10 +347,66 @@
                     Stamp newStampY = binaryOpLogicNode.getSucceedingStampForY(negated);
                     registerNewStamp(y, newStampY, guard, undoOperations);
                 }
+                if (condition instanceof IntegerEqualsNode && guard instanceof FixedGuardNode) {
+                    if (y.isConstant() && x instanceof AndNode) {
+                        AndNode and = (AndNode) x;
+                        if (and.getY() == y) {
+                            /*
+                             * This 'and' proves something about some of the bits in and.getX().
+                             * It's equivalent to or'ing in the mask value since those values are
+                             * known to be set.
+                             */
+                            BinaryOp<Or> op = ArithmeticOpTable.forStamp(x.stamp()).getOr();
+                            IntegerStamp newStampX = (IntegerStamp) op.foldStamp(and.getX().stamp(), y.stamp());
+                            registerNewStamp(and.getX(), newStampX, guard, undoOperations);
+                            foldPendingTest(negated, guard, and.getX(), newStampX);
+                        }
+                    }
+                }
             }
+            pendingTests.push(new PendingTest(condition, negated, guard));
             registerCondition(condition, negated, guard, undoOperations);
         }
 
+        private void foldPendingTest(boolean negated, ValueNode guard, ValueNode originalX, IntegerStamp newStampX) {
+            for (PendingTest pending : pendingTests) {
+                if (pending.condition instanceof BinaryOpLogicNode) {
+                    TriState result = TriState.UNKNOWN;
+                    BinaryOpLogicNode binaryOpLogicNode = (BinaryOpLogicNode) pending.condition;
+                    ValueNode x = binaryOpLogicNode.getX();
+                    ValueNode y = binaryOpLogicNode.getY();
+                    if (binaryOpLogicNode.getX() == originalX) {
+                        result = binaryOpLogicNode.tryFold(newStampX, binaryOpLogicNode.getY().stamp());
+                    } else if (binaryOpLogicNode instanceof IntegerEqualsNode && y.isConstant() && x instanceof AndNode) {
+                        AndNode and2 = (AndNode) x;
+                        if (and2.getY() == y) {
+                            BinaryOp<And> andOp = ArithmeticOpTable.forStamp(originalX.stamp()).getAnd();
+                            result = binaryOpLogicNode.tryFold(andOp.foldStamp(newStampX, y.stamp()), y.stamp());
+                        }
+                    }
+                    /*
+                     * If the failure of the pending guard would also be triggered by the failure of
+                     * this guard, then it's safe to replace the earlier guard with this guard.
+                     */
+                    if (result.isKnown()) {
+                        if (pending.guard instanceof FixedGuardNode && guard instanceof FixedGuardNode) {
+                            FixedGuardNode fix = (FixedGuardNode) pending.guard;
+                            FixedGuardNode thisGuard = (FixedGuardNode) guard;
+                            if (fix.getAction() == thisGuard.getAction() && fix.getReason() == thisGuard.getReason() && fix.getSpeculation() == thisGuard.getSpeculation()) {
+                                if (pending.negated == negated && result.isTrue() || pending.negated && result.isFalse()) {
+                                    assert !negated;
+                                    FixedGuardNode newFix = (FixedGuardNode) thisGuard.copyWithInputs();
+                                    guard.graph().replaceFixedWithFixed(fix, newFix);
+                                    guard.replaceAtUsages(newFix);
+                                    guard.graph().removeFixed(thisGuard);
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
         private void registerCondition(LogicNode condition, boolean negated, ValueNode guard, List<Runnable> undoOperations) {
             registerNewStamp(condition, negated ? StampFactory.contradiction() : StampFactory.tautology(), guard, undoOperations);
         }