changeset 4453:c0430421d43d

bugfixes for inlining multiple methods
author Christian Haeubl <christian.haeubl@oracle.com>
date Fri, 27 Jan 2012 21:17:33 -0800
parents b225da954a32
children 008a5ea590ca
files graal/com.oracle.max.cri/src/com/oracle/max/cri/ri/RiTypeProfile.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/util/InliningUtil.java graal/com.oracle.max.graal.hotspot/src/com/oracle/max/graal/hotspot/ri/HotSpotMethodData.java graal/com.oracle.max.graal.hotspot/src/com/oracle/max/graal/hotspot/ri/HotSpotRuntime.java graal/com.oracle.max.graal.nodes/src/com/oracle/max/graal/nodes/extended/ObjectClassNode.java graal/com.oracle.max.graal.nodes/src/com/oracle/max/graal/nodes/extended/READCLASSNODE.JAVA graal/com.oracle.max.graal.nodes/src/com/oracle/max/graal/nodes/java/IsTypeNode.java
diffstat 7 files changed, 80 insertions(+), 69 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.max.cri/src/com/oracle/max/cri/ri/RiTypeProfile.java	Fri Jan 27 18:16:32 2012 -0800
+++ b/graal/com.oracle.max.cri/src/com/oracle/max/cri/ri/RiTypeProfile.java	Fri Jan 27 21:17:33 2012 -0800
@@ -25,9 +25,9 @@
 import java.io.*;
 
 /**
- * This profile object represents the type profile of one call site, cast or instanceof instruction. The precision of
- * the supplied values may vary, but a runtime that provides this information should be aware that it will be used to
- * guide performance-critical decisions like speculative inlining, etc.
+ * This profile object represents the type profile at a specific BCI. The precision of the supplied values may vary,
+ * but a runtime that provides this information should be aware that it will be used to guide performance-critical
+ * decisions like speculative inlining, etc.
  */
 public final class RiTypeProfile implements Serializable {
     /**
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/util/InliningUtil.java	Fri Jan 27 18:16:32 2012 -0800
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/util/InliningUtil.java	Fri Jan 27 21:17:33 2012 -0800
@@ -156,10 +156,12 @@
         public void inline(StructuredGraph graph, GraalRuntime runtime, InliningCallback callback) {
             // receiver null check must be before the type check
             InliningUtil.receiverNullCheck(invoke);
-            ObjectClassNode objectClass = new ObjectClassNode(invoke.callTarget().receiver());
+            ReadClassNode objectClass = graph.add(new ReadClassNode(invoke.callTarget().receiver()));
             IsTypeNode isTypeNode = graph.unique(new IsTypeNode(objectClass, type));
             FixedGuardNode guard = graph.add(new FixedGuardNode(isTypeNode));
             assert invoke.predecessor() != null;
+
+            graph.addBeforeFixed(invoke.node(), objectClass);
             graph.addBeforeFixed(invoke.node(), guard);
 
             if (GraalOptions.TraceInlining) {
@@ -228,11 +230,15 @@
                 }
             }
 
+            // TODO (ch) do not create merge nodes if not necessary. Otherwise GraphVisualizer seems to loose half of the phase outputs?
+
             // create a separate block for each invoked method
-            BeginNode[] successorMethods = new BeginNode[numberOfMethods];
+            MergeNode[] successorMethods = new MergeNode[numberOfMethods];
             for (int i = 0; i < numberOfMethods; i++) {
                 Invoke duplicatedInvoke = duplicateInvoke(invoke);
-                successorMethods[i] = BeginNode.begin(duplicatedInvoke.node());
+                MergeNode calleeEntryNode = graph.add(new MergeNode());
+                calleeEntryNode.setNext(duplicatedInvoke.node());
+                successorMethods[i] = calleeEntryNode;
 
                 if (merge != null) {
                     EndNode endNode = graph.add(new EndNode());
@@ -250,19 +256,24 @@
             }
 
             // create a cascade of ifs with the type checks
-            ObjectClassNode objectClassNode = graph.add(new ObjectClassNode(invoke.callTarget().receiver()));
+            ReadClassNode objectClassNode = graph.add(new ReadClassNode(invoke.callTarget().receiver()));
+            graph.addBeforeFixed(invoke.node(), objectClassNode);
 
             int lastIndex = types.length - 1;
-            BeginNode tsux = successorMethods[typesToConcretes[lastIndex]];
-            IsTypeNode isTypeNode = graph.add(new IsTypeNode(objectClassNode, types[lastIndex]));
+            MergeNode tsux = successorMethods[typesToConcretes[lastIndex]];
+            IsTypeNode isTypeNode = graph.unique(new IsTypeNode(objectClassNode, types[lastIndex]));
+            EndNode endNode = graph.add(new EndNode());
             FixedGuardNode guardNode = graph.add(new FixedGuardNode(isTypeNode));
-            guardNode.setNext(tsux);
+            guardNode.setNext(endNode);
+            tsux.addEnd(endNode);
 
             FixedNode nextNode = guardNode;
             for (int i = lastIndex - 1; i >= 0; i--) {
                 tsux = successorMethods[typesToConcretes[i]];
-                isTypeNode = graph.add(new IsTypeNode(objectClassNode, types[i]));
-                nextNode = graph.add(new IfNode(isTypeNode, tsux, nextNode, probabilities[i]));
+                isTypeNode = graph.unique(new IsTypeNode(objectClassNode, types[i]));
+                endNode = graph.add(new EndNode());
+                nextNode = graph.add(new IfNode(isTypeNode, endNode, nextNode, probabilities[i]));
+                tsux.addEnd(endNode);
             }
 
             // replace the original invocation with a cascade of if nodes and replace the usages of invoke with the return value (phi or duplicatedInvoke)
@@ -272,7 +283,7 @@
 
             // do the actual inlining for every invocation
             for (int i = 0; i < successorMethods.length; i++) {
-                BeginNode node = successorMethods[i];
+                MergeNode node = successorMethods[i];
                 Invoke invokeForInlining = (Invoke) node.next();
                 StructuredGraph calleeGraph = getGraph(invokeForInlining, concretes.get(i), callback);
                 InliningUtil.inline(invokeForInlining, calleeGraph, false);
@@ -425,7 +436,6 @@
                         return null;
                     } else {
                         // TODO (ch) sort types by probability
-
                         // determine concrete methods and map type to specific method
                         ArrayList<RiResolvedMethod> concreteMethods = new ArrayList<>();
                         int[] typesToConcretes = new int[types.length];
--- a/graal/com.oracle.max.graal.hotspot/src/com/oracle/max/graal/hotspot/ri/HotSpotMethodData.java	Fri Jan 27 18:16:32 2012 -0800
+++ b/graal/com.oracle.max.graal.hotspot/src/com/oracle/max/graal/hotspot/ri/HotSpotMethodData.java	Fri Jan 27 21:17:33 2012 -0800
@@ -322,6 +322,8 @@
 
         @Override
         public RiTypeProfile getTypeProfile(HotSpotMethodData data, int position) {
+            // TODO (ch) detect polymorphic case and return null and document interface accordingly
+            // is it really the best solution to return null?
             int typeProfileWidth = config.typeProfileWidth;
 
             RiResolvedType[] sparseTypes = new RiResolvedType[typeProfileWidth];
--- a/graal/com.oracle.max.graal.hotspot/src/com/oracle/max/graal/hotspot/ri/HotSpotRuntime.java	Fri Jan 27 18:16:32 2012 -0800
+++ b/graal/com.oracle.max.graal.hotspot/src/com/oracle/max/graal/hotspot/ri/HotSpotRuntime.java	Fri Jan 27 21:17:33 2012 -0800
@@ -313,13 +313,12 @@
         } else if (n instanceof ArrayHeaderSizeNode) {
             ArrayHeaderSizeNode arrayHeaderSize = (ArrayHeaderSizeNode) n;
             graph.replaceFloating(arrayHeaderSize, ConstantNode.forLong(config.getArrayOffset(arrayHeaderSize.elementKind()), n.graph()));
-        } else if (n instanceof ObjectClassNode) {
-            ObjectClassNode objectClassNode = (ObjectClassNode) n;
+        } else if (n instanceof ReadClassNode) {
+            ReadClassNode objectClassNode = (ReadClassNode) n;
             LocationNode location = LocationNode.create(LocationNode.FINAL_LOCATION, CiKind.Object, config.hubOffset, graph);
-            FloatingReadNode memoryRead = graph.add(new FloatingReadNode(CiKind.Object, objectClassNode.object(), null, location));
-            // TODO (ch) this fails because ObjectClass is only used as an input value
-            //memoryRead.setGuard((GuardNode) tool.createGuard(graph.unique(new NullCheckNode(objectClassNode.object(), false))));
-            graph.replaceFloating(objectClassNode, memoryRead);
+            ReadNode memoryRead = graph.add(new ReadNode(CiKind.Object, objectClassNode.object(), location));
+            memoryRead.setGuard((GuardNode) tool.createGuard(graph.unique(new NullCheckNode(objectClassNode.object(), false))));
+            graph.replaceFixed(objectClassNode, memoryRead);
         }
     }
 
--- a/graal/com.oracle.max.graal.nodes/src/com/oracle/max/graal/nodes/extended/ObjectClassNode.java	Fri Jan 27 18:16:32 2012 -0800
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,48 +0,0 @@
-/*
- * 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.max.graal.nodes.extended;
-
-import com.oracle.max.cri.ci.*;
-import com.oracle.max.graal.cri.*;
-import com.oracle.max.graal.nodes.*;
-import com.oracle.max.graal.nodes.calc.*;
-import com.oracle.max.graal.nodes.spi.*;
-import com.oracle.max.graal.nodes.type.*;
-
-public final class ObjectClassNode extends FloatingNode implements Lowerable {
-    @Input private ValueNode object;
-
-    public ValueNode object() {
-        return object;
-    }
-
-    public ObjectClassNode(ValueNode object) {
-        super(StampFactory.forKind(CiKind.Object));
-        this.object = object;
-    }
-
-    @Override
-    public void lower(CiLoweringTool tool) {
-        tool.getRuntime().lower(this, tool);
-    }
-}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/graal/com.oracle.max.graal.nodes/src/com/oracle/max/graal/nodes/extended/READCLASSNODE.JAVA	Fri Jan 27 21:17:33 2012 -0800
@@ -0,0 +1,48 @@
+/*
+ * 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.max.graal.nodes.extended;
+
+import com.oracle.max.cri.ci.*;
+import com.oracle.max.graal.cri.*;
+import com.oracle.max.graal.nodes.*;
+import com.oracle.max.graal.nodes.spi.*;
+import com.oracle.max.graal.nodes.type.*;
+
+// TODO (ch) this should be a FloatingNode but Lowering crashes in that case
+public final class ReadClassNode extends FixedWithNextNode implements Lowerable {
+    @Input private ValueNode object;
+
+    public ValueNode object() {
+        return object;
+    }
+
+    public ReadClassNode(ValueNode object) {
+        super(StampFactory.forKind(CiKind.Object));
+        this.object = object;
+    }
+
+    @Override
+    public void lower(CiLoweringTool tool) {
+        tool.getRuntime().lower(this, tool);
+    }
+}
--- a/graal/com.oracle.max.graal.nodes/src/com/oracle/max/graal/nodes/java/IsTypeNode.java	Fri Jan 27 18:16:32 2012 -0800
+++ b/graal/com.oracle.max.graal.nodes/src/com/oracle/max/graal/nodes/java/IsTypeNode.java	Fri Jan 27 21:17:33 2012 -0800
@@ -71,7 +71,7 @@
 
     @Override
     public ValueNode canonical(CanonicalizerTool tool) {
-        RiResolvedType exactType = objectClass() instanceof ObjectClassNode ? ((ObjectClassNode) objectClass()).object().exactType() : null;
+        RiResolvedType exactType = objectClass() instanceof ReadClassNode ? ((ReadClassNode) objectClass()).object().exactType() : null;
         if (exactType != null) {
             return ConstantNode.forBoolean(exactType == type(), graph());
         }