Mercurial > hg > truffle

changeset 13604:bd21ee1a874c

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
fix recursion problem around macro nodes (e.g. array copy)
author Bernhard Urban <bernhard.urban@jku.at>
date Fri, 10 Jan 2014 20:16:31 +0100
parents aaaeef103625
children 3def004aaa2d
files graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ArrayCopyNode.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/stubs/GraphKit.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/spi/Replacements.java graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ObjectAccessTest.java graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PointerTest.java graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/WordTest.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java
diffstat 7 files changed, 57 insertions(+), 25 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ArrayCopyNode.java	Fri Jan 10 15:26:01 2014 +0100
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ArrayCopyNode.java	Fri Jan 10 20:16:31 2014 +0100
@@ -107,7 +107,7 @@
             final ResolvedJavaMethod snippetMethod = tool.getMetaAccess().lookupJavaMethod(ArrayCopySnippets.genericArraycopySnippet);
             snippetGraph = null;
             try (Scope s = Debug.scope("ArrayCopySnippet", snippetMethod)) {
-                snippetGraph = replacements.getSnippet(snippetMethod).copy();
+                snippetGraph = replacements.getSnippet(snippetMethod, getTargetMethod()).copy();
             } catch (Throwable e) {
                 throw Debug.handle(e);
             }
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/stubs/GraphKit.java	Fri Jan 10 15:26:01 2014 +0100
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/stubs/GraphKit.java	Fri Jan 10 20:16:31 2014 +0100
@@ -158,7 +158,7 @@
     public void inline(InvokeNode invoke) {
         ResolvedJavaMethod method = ((MethodCallTargetNode) invoke.callTarget()).targetMethod();
         ReplacementsImpl repl = new ReplacementsImpl(providers, new Assumptions(false), providers.getCodeCache().getTarget());
-        StructuredGraph calleeGraph = repl.makeGraph(method, null, null, false);
+        StructuredGraph calleeGraph = repl.makeGraph(method, null, method, null, false, false);
         InliningUtil.inline(invoke, calleeGraph, false);
     }
 }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/spi/Replacements.java	Fri Jan 10 15:26:01 2014 +0100
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/spi/Replacements.java	Fri Jan 10 20:16:31 2014 +0100
@@ -42,6 +42,16 @@
     StructuredGraph getSnippet(ResolvedJavaMethod method);
 
     /**
+     * Gets the snippet graph derived from a given method.
+     * 
+     * @param recursiveEntry if the snippet contains a call to this method, it's considered as
+     *            recursive call and won't be processed for {@linkplain MethodSubstitution
+     *            substitutions} or {@linkplain MacroSubstitution macro nodes}.
+     * @return the snippet graph, if any, that is derived from {@code method}
+     */
+    StructuredGraph getSnippet(ResolvedJavaMethod method, ResolvedJavaMethod recursiveEntry);
+
+    /**
      * Registers a method as snippet.
      */
     void registerSnippet(ResolvedJavaMethod method);
--- a/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ObjectAccessTest.java	Fri Jan 10 15:26:01 2014 +0100
+++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ObjectAccessTest.java	Fri Jan 10 20:16:31 2014 +0100
@@ -54,7 +54,7 @@
     @Override
     protected StructuredGraph parse(Method m) {
         ResolvedJavaMethod resolvedMethod = getMetaAccess().lookupJavaMethod(m);
-        return installer.makeGraph(resolvedMethod, null, inliningPolicy.get(), false);
+        return installer.makeGraph(resolvedMethod, null, resolvedMethod, inliningPolicy.get(), false, false);
     }
 
     @Test
--- a/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PointerTest.java	Fri Jan 10 15:26:01 2014 +0100
+++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PointerTest.java	Fri Jan 10 20:16:31 2014 +0100
@@ -60,7 +60,7 @@
     @Override
     protected StructuredGraph parse(Method m) {
         ResolvedJavaMethod resolvedMethod = getMetaAccess().lookupJavaMethod(m);
-        return installer.makeGraph(resolvedMethod, null, inliningPolicy.get(), false);
+        return installer.makeGraph(resolvedMethod, null, resolvedMethod, inliningPolicy.get(), false, false);
     }
 
     @Test
--- a/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/WordTest.java	Fri Jan 10 15:26:01 2014 +0100
+++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/WordTest.java	Fri Jan 10 20:16:31 2014 +0100
@@ -49,7 +49,7 @@
     @Override
     protected StructuredGraph parse(Method m) {
         ResolvedJavaMethod resolvedMethod = getMetaAccess().lookupJavaMethod(m);
-        return installer.makeGraph(resolvedMethod, null, inliningPolicy.get(), false);
+        return installer.makeGraph(resolvedMethod, null, resolvedMethod, inliningPolicy.get(), false, false);
     }
 
     @LongTest
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java	Fri Jan 10 15:26:01 2014 +0100
+++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java	Fri Jan 10 20:16:31 2014 +0100
@@ -84,15 +84,19 @@
     private static final boolean UseSnippetGraphCache = Boolean.parseBoolean(System.getProperty("graal.useSnippetGraphCache", "true"));
     private static final DebugTimer SnippetPreparationTime = Debug.timer("SnippetPreparationTime");
 
+    public StructuredGraph getSnippet(ResolvedJavaMethod method) {
+        return getSnippet(method, null);
+    }
+
     @Override
-    public StructuredGraph getSnippet(ResolvedJavaMethod method) {
+    public StructuredGraph getSnippet(ResolvedJavaMethod method, ResolvedJavaMethod recursiveEntry) {
         assert method.getAnnotation(Snippet.class) != null : "Snippet must be annotated with @" + Snippet.class.getSimpleName();
         assert !Modifier.isAbstract(method.getModifiers()) && !Modifier.isNative(method.getModifiers()) : "Snippet must not be abstract or native";
 
         StructuredGraph graph = UseSnippetGraphCache ? graphs.get(method) : null;
         if (graph == null) {
             try (TimerCloseable a = SnippetPreparationTime.start()) {
-                StructuredGraph newGraph = makeGraph(method, null, inliningPolicy(method), method.getAnnotation(Snippet.class).removeAllFrameStates());
+                StructuredGraph newGraph = makeGraph(method, recursiveEntry, recursiveEntry, inliningPolicy(method), method.getAnnotation(Snippet.class).removeAllFrameStates(), false);
                 Debug.metric("SnippetNodeCount[" + method.getName() + "]").add(newGraph.getNodeCount());
                 if (!UseSnippetGraphCache) {
                     return newGraph;
@@ -127,7 +131,7 @@
         }
         StructuredGraph graph = graphs.get(substitute);
         if (graph == null) {
-            graphs.putIfAbsent(substitute, makeGraph(substitute, original, inliningPolicy(substitute), false));
+            graphs.putIfAbsent(substitute, makeGraph(substitute, original, substitute, inliningPolicy(substitute), false, true));
             graph = graphs.get(substitute);
         }
         return graph;
@@ -259,15 +263,16 @@
      * @param policy the inlining policy to use during preprocessing
      * @param removeAllFrameStates removes all frame states from side effecting instructions
      */
-    public StructuredGraph makeGraph(ResolvedJavaMethod method, ResolvedJavaMethod original, SnippetInliningPolicy policy, boolean removeAllFrameStates) {
-        return createGraphMaker(method, original).makeGraph(policy, removeAllFrameStates);
+    public StructuredGraph makeGraph(ResolvedJavaMethod method, ResolvedJavaMethod original, ResolvedJavaMethod recursiveEntry, SnippetInliningPolicy policy, boolean removeAllFrameStates,
+                    boolean isMethodSubstitution) {
+        return createGraphMaker(method, original, recursiveEntry, isMethodSubstitution).makeGraph(policy, removeAllFrameStates);
     }
 
     /**
      * Can be overridden to return an object that specializes various parts of graph preprocessing.
      */
-    protected GraphMaker createGraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod original) {
-        return new GraphMaker(substitute, original);
+    protected GraphMaker createGraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod original, ResolvedJavaMethod recursiveEntry, boolean isMethodSubstitution) {
+        return new GraphMaker(substitute, original, recursiveEntry, isMethodSubstitution);
     }
 
     /**
@@ -279,21 +284,32 @@
      * Creates and preprocesses a graph for a replacement.
      */
     protected class GraphMaker {
-
         /**
          * The method for which a graph is being created.
          */
         protected final ResolvedJavaMethod method;
 
         /**
-         * The original method if {@link #method} is a {@linkplain MethodSubstitution substitution}
-         * otherwise null.
+         * The method which is used when a call to {@link #recursiveEntry} is found.
+         */
+        protected final ResolvedJavaMethod substitutedMethod;
+
+        /**
+         * The method which is used to detect a recursive call.
          */
-        protected final ResolvedJavaMethod original;
+        protected final ResolvedJavaMethod recursiveEntry;
 
-        protected GraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod original) {
+        /**
+         * Controls if FrameStates should be removed or processed with the
+         * {@link SnippetFrameStateCleanupPhase}.
+         */
+        protected final boolean isMethodSubstitution;
+
+        protected GraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod substitutedMethod, ResolvedJavaMethod recursiveEntry, boolean isMethodSubstitution) {
             this.method = substitute;
-            this.original = original;
+            this.substitutedMethod = substitutedMethod;
+            this.recursiveEntry = recursiveEntry;
+            this.isMethodSubstitution = isMethodSubstitution;
         }
 
         public StructuredGraph makeGraph(final SnippetInliningPolicy policy, final boolean removeAllFrameStates) {
@@ -323,7 +339,7 @@
             }
             new ConvertDeoptimizeToGuardPhase().apply(graph);
 
-            if (original == null) {
+            if (!isMethodSubstitution) {
                 if (removeAllFrameStates) {
                     for (Node node : graph.getNodes()) {
                         if (node instanceof StateSplit) {
@@ -403,18 +419,20 @@
         }
 
         private StructuredGraph buildGraph(final ResolvedJavaMethod methodToParse, final SnippetInliningPolicy policy) {
-            assert !Modifier.isAbstract(methodToParse.getModifiers()) && !Modifier.isNative(methodToParse.getModifiers()) : methodToParse;
+            assert isInlinableSnippet(methodToParse) : methodToParse;
             final StructuredGraph graph = buildInitialGraph(methodToParse);
             try (Scope s = Debug.scope("buildGraph", graph)) {
 
                 for (MethodCallTargetNode callTarget : graph.getNodes(MethodCallTargetNode.class)) {
                     ResolvedJavaMethod callee = callTarget.targetMethod();
-                    if (callee == method) {
-                        final StructuredGraph originalGraph = buildInitialGraph(original);
-                        InliningUtil.inline(callTarget.invoke(), originalGraph, true);
+                    if (callee == recursiveEntry) {
+                        if (isInlinableSnippet(substitutedMethod)) {
+                            final StructuredGraph originalGraph = buildInitialGraph(substitutedMethod);
+                            InliningUtil.inline(callTarget.invoke(), originalGraph, true);
 
-                        Debug.dump(graph, "after inlining %s", callee);
-                        afterInline(graph, originalGraph, null);
+                            Debug.dump(graph, "after inlining %s", callee);
+                            afterInline(graph, originalGraph, null);
+                        }
                     } else {
                         Class<? extends FixedWithNextNode> macroNodeClass = InliningUtil.getMacroNodeClass(ReplacementsImpl.this, callee);
                         if (macroNodeClass != null) {
@@ -457,6 +475,10 @@
         }
     }
 
+    private static boolean isInlinableSnippet(final ResolvedJavaMethod methodToParse) {
+        return !Modifier.isAbstract(methodToParse.getModifiers()) && !Modifier.isNative(methodToParse.getModifiers());
+    }
+
     private static String originalName(Method substituteMethod, String methodSubstitution) {
         if (methodSubstitution.isEmpty()) {
             return substituteMethod.getName();