changeset 10541:9599e1a01812

Merge.
author Thomas Wuerthinger <thomas.wuerthinger@oracle.com>
date Wed, 26 Jun 2013 15:22:21 +0200
parents 0ba44a5a8420 (current diff) 2faa1e7ef4f3 (diff)
children 554f67e4ff3f 6eb8d63cea34
files
diffstat 5 files changed, 155 insertions(+), 28 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/ConditionalEliminationTest.java	Wed Jun 26 15:22:21 2013 +0200
@@ -0,0 +1,92 @@
+/*
+ * Copyright (c) 2011, 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.*;
+
+import com.oracle.graal.phases.common.*;
+
+/**
+ * Collection of tests for {@link ConditionalEliminationPhase} including those that triggered bugs
+ * in this phase.
+ */
+public class ConditionalEliminationTest extends GraalCompilerTest {
+
+    static class Entry {
+
+        final String name;
+
+        public Entry(String name) {
+            this.name = name;
+        }
+    }
+
+    static class EntryWithNext extends Entry {
+
+        public EntryWithNext(String name, Entry next) {
+            super(name);
+            this.next = next;
+        }
+
+        final Entry next;
+    }
+
+    public static Entry search(Entry start, String name, Entry alternative) {
+        Entry current = start;
+        do {
+            while (current instanceof EntryWithNext) {
+                if (name != null && current.name == name) {
+                    current = null;
+                } else {
+                    Entry next = ((EntryWithNext) current).next;
+                    current = next;
+                }
+            }
+
+            if (current != null) {
+                if (current.name.equals(name)) {
+                    return current;
+                }
+            }
+            if (current == alternative) {
+                return null;
+            }
+            current = alternative;
+
+        } while (true);
+    }
+
+    /**
+     * This test presents a code pattern that triggered a bug where a (non-eliminated) checkcast
+     * caused an enclosing instanceof (for the same object and target type) to be incorrectly
+     * eliminated.
+     */
+    @Test
+    public void testReanchoringIssue() {
+        Entry end = new Entry("end");
+        EntryWithNext e1 = new EntryWithNext("e1", end);
+        EntryWithNext e2 = new EntryWithNext("e2", e1);
+
+        test("search", e2, "e3", new Entry("e4"));
+    }
+}
--- a/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/GraalCompilerTest.java	Wed Jun 26 15:22:11 2013 +0200
+++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/GraalCompilerTest.java	Wed Jun 26 15:22:21 2013 +0200
@@ -451,6 +451,9 @@
                     @Override
                     public InstalledCode call() throws Exception {
                         InstalledCode code = addMethod(method, compResult);
+                        if (code == null) {
+                            throw new GraalInternalError("Could not install code for " + MetaUtil.format("%H.%n(%p)", method));
+                        }
                         if (Debug.isDumpEnabled()) {
                             Debug.dump(new Object[]{compResult, code}, "After code installation");
                         }
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/ConditionalEliminationPhase.java	Wed Jun 26 15:22:11 2013 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/ConditionalEliminationPhase.java	Wed Jun 26 15:22:21 2013 +0200
@@ -27,6 +27,7 @@
 import com.oracle.graal.api.meta.*;
 import com.oracle.graal.debug.*;
 import com.oracle.graal.graph.*;
+import com.oracle.graal.graph.iterators.*;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.nodes.PhiNode.PhiType;
 import com.oracle.graal.nodes.calc.*;
@@ -493,12 +494,52 @@
             } else if (node instanceof IfNode) {
                 IfNode ifNode = (IfNode) node;
                 LogicNode compare = ifNode.condition();
-                LogicNode replacement = evaluateCondition(compare, trueConstant, falseConstant);
+
+                LogicNode replacement = null;
+                ValueNode replacementAnchor = null;
+                AbstractBeginNode survivingSuccessor = null;
+                if (state.trueConditions.containsKey(compare)) {
+                    replacement = trueConstant;
+                    replacementAnchor = state.trueConditions.get(compare);
+                    survivingSuccessor = ifNode.trueSuccessor();
+                } else if (state.falseConditions.containsKey(compare)) {
+                    replacement = falseConstant;
+                    replacementAnchor = state.falseConditions.get(compare);
+                    survivingSuccessor = ifNode.falseSuccessor();
+                } else {
+                    replacement = evaluateCondition(compare, trueConstant, falseConstant);
+                    if (replacement != null) {
+                        if (replacement == trueConstant) {
+                            survivingSuccessor = ifNode.trueSuccessor();
+                        } else {
+                            assert replacement == falseConstant;
+                            survivingSuccessor = ifNode.falseSuccessor();
+                        }
+                    }
+                }
 
                 if (replacement != null) {
-                    ifNode.setCondition(replacement);
-                    if (compare.usages().isEmpty()) {
-                        GraphUtil.killWithUnusedFloatingInputs(compare);
+                    NodeIterable<Node> anchored = survivingSuccessor.anchored();
+                    if (!anchored.isEmpty() && replacementAnchor == null) {
+                        // Cannot simplify an IfNode unless we have a new anchor point
+                        // for any nodes currently anchored to the surviving branch
+                    } else {
+                        if (!anchored.isEmpty()) {
+                            // Ideally we'd simply want to re-anchor to replacementAnchor. However,
+                            // this can cause guards currently anchored to the surviving branch
+                            // to float too high in the graph. So, we insert a new anchor between
+                            // the guards and replacementAnchor.
+                            ValueAnchorNode valueAnchor = graph.add(new ValueAnchorNode());
+                            for (Node a : anchored.snapshot()) {
+                                a.replaceFirstInput(survivingSuccessor, valueAnchor);
+                            }
+                            valueAnchor.addAnchoredNode(replacementAnchor);
+                            graph.addBeforeFixed(ifNode, valueAnchor);
+                        }
+                        ifNode.setCondition(replacement);
+                        if (compare.usages().isEmpty()) {
+                            GraphUtil.killWithUnusedFloatingInputs(compare);
+                        }
                     }
                 }
             } else if (node instanceof AbstractEndNode) {
@@ -522,5 +563,4 @@
             }
         }
     }
-
 }
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/TailDuplicationPhase.java	Wed Jun 26 15:22:11 2013 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/TailDuplicationPhase.java	Wed Jun 26 15:22:21 2013 +0200
@@ -50,10 +50,8 @@
     /*
      * Various metrics on the circumstances in which tail duplication was/wasn't performed.
      */
-    private static final DebugMetric metricDuplicationEnd = Debug.metric("DuplicationEnd");
-    private static final DebugMetric metricDuplicationEndPerformed = Debug.metric("DuplicationEndPerformed");
-    private static final DebugMetric metricDuplicationOther = Debug.metric("DuplicationOther");
-    private static final DebugMetric metricDuplicationOtherPerformed = Debug.metric("DuplicationOtherPerformed");
+    private static final DebugMetric metricDuplicationConsidered = Debug.metric("DuplicationConsidered");
+    private static final DebugMetric metricDuplicationPerformed = Debug.metric("DuplicationPerformed");
 
     /**
      * This interface is used by tail duplication to let clients decide if tail duplication should
@@ -171,20 +169,11 @@
             fixedCount++;
         }
         if (fixedCount > 1) {
-            if (fixed instanceof AbstractEndNode && !(((AbstractEndNode) fixed).merge() instanceof LoopBeginNode)) {
-                metricDuplicationEnd.increment();
-                if (decision.doTransform(merge, fixedCount)) {
-                    metricDuplicationEndPerformed.increment();
-                    new DuplicationOperation(merge, replacements).duplicate(phaseContext);
-                    return true;
-                }
-            } else if (merge.stateAfter() != null) {
-                metricDuplicationOther.increment();
-                if (decision.doTransform(merge, fixedCount)) {
-                    metricDuplicationOtherPerformed.increment();
-                    new DuplicationOperation(merge, replacements).duplicate(phaseContext);
-                    return true;
-                }
+            metricDuplicationConsidered.increment();
+            if (decision.doTransform(merge, fixedCount)) {
+                metricDuplicationPerformed.increment();
+                new DuplicationOperation(merge, replacements).duplicate(phaseContext);
+                return true;
             }
         }
         return false;
--- a/mxtool/mx.py	Wed Jun 26 15:22:11 2013 +0200
+++ b/mxtool/mx.py	Wed Jun 26 15:22:21 2013 +0200
@@ -3287,17 +3287,20 @@
 _argParser = ArgParser()
 
 def _findPrimarySuite():
+    def is_suite_dir(d):
+        mxDir = join(d, 'mx')
+        if exists(mxDir) and isdir(mxDir) and exists(join(mxDir, 'projects')):
+            return dirname(mxDir)
+
     # try current working directory first
-    mxDir = join(os.getcwd(), 'mx')
-    if exists(mxDir) and isdir(mxDir):
-        return dirname(mxDir)
+    if is_suite_dir(os.getcwd()):
+        return os.getcwd()
 
     # now search path of my executable
     me = sys.argv[0]
     parent = dirname(me)
     while parent:
-        mxDir = join(parent, 'mx')
-        if exists(mxDir) and isdir(mxDir):
+        if is_suite_dir(parent):
             return parent
         parent = dirname(parent)
     return None