Mercurial > hg > truffle

changeset 7901:93a133fc03ce

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
clear frame states in snippets and replace with sentries
author Lukas Stadler <lukas.stadler@jku.at>
date Wed, 27 Feb 2013 18:28:09 +0100
parents 30d754a0e87c
children 14fedab0419e
files graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/FrameState.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningUtil.java graal/com.oracle.graal.snippets.test/src/com/oracle/graal/snippets/WordTest.java graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/SnippetFrameStateCleanupPhase.java graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/SnippetInstaller.java
diffstat 5 files changed, 88 insertions(+), 24 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/FrameState.java	Wed Feb 27 17:53:21 2013 +0100
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/FrameState.java	Wed Feb 27 18:28:09 2013 +0100
@@ -68,6 +68,12 @@
      */
     public static final int AFTER_EXCEPTION_BCI = -3;
 
+    /**
+     * This BCI should be used for frame states that cannot be the target of a deoptimization, like
+     * snippet frame states.
+     */
+    public static final int INVALID_FRAMESTATE_BCI = -5;
+
     @Input private FrameState outerFrameState;
 
     @Input private final NodeInputList<ValueNode> values;
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningUtil.java	Wed Feb 27 17:53:21 2013 +0100
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningUtil.java	Wed Feb 27 18:28:09 2013 +0100
@@ -1122,7 +1122,7 @@
                 } else {
                     // only handle the outermost frame states
                     if (frameState.outerFrameState() == null) {
-                        assert frameState.method() == inlineGraph.method();
+                        assert frameState.bci == FrameState.INVALID_FRAMESTATE_BCI || frameState.method() == inlineGraph.method();
                         if (outerFrameState == null) {
                             outerFrameState = stateAfter.duplicateModified(invoke.bci(), stateAfter.rethrowException(), invoke.node().kind());
                             outerFrameState.setDuringCall(true);
--- a/graal/com.oracle.graal.snippets.test/src/com/oracle/graal/snippets/WordTest.java	Wed Feb 27 17:53:21 2013 +0100
+++ b/graal/com.oracle.graal.snippets.test/src/com/oracle/graal/snippets/WordTest.java	Wed Feb 27 18:28:09 2013 +0100
@@ -51,7 +51,7 @@
     @Override
     protected StructuredGraph parse(Method m) {
         ResolvedJavaMethod resolvedMethod = runtime.lookupJavaMethod(m);
-        return installer.makeGraph(resolvedMethod, inliningPolicy.get(), false);
+        return installer.makeGraph(resolvedMethod, inliningPolicy.get());
     }
 
     @Test
--- a/graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/SnippetFrameStateCleanupPhase.java	Wed Feb 27 17:53:21 2013 +0100
+++ b/graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/SnippetFrameStateCleanupPhase.java	Wed Feb 27 18:28:09 2013 +0100
@@ -22,30 +22,98 @@
  */
 package com.oracle.graal.snippets;
 
-import com.oracle.graal.graph.*;
+import java.util.*;
+
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.nodes.util.*;
 import com.oracle.graal.phases.*;
+import com.oracle.graal.phases.graph.*;
+import com.oracle.graal.phases.graph.ReentrantNodeIterator.LoopInfo;
+import com.oracle.graal.phases.graph.ReentrantNodeIterator.NodeIteratorClosure;
 
 /**
  * Removes frame states from {@linkplain StateSplit#hasSideEffect() non-side-effecting} nodes in a
  * snippet.
+ * 
+ * The frame states of side-effecting nodes are replaced with
+ * {@linkplain FrameState#INVALID_FRAMESTATE_BCI invalid} frame states. Loops that contain invalid
+ * frame states are also assigned an invalid frame state.
+ * 
+ * The invalid frame states ensure that no deoptimization to a snippet frame state will happen.
  */
 public class SnippetFrameStateCleanupPhase extends Phase {
 
     @Override
     protected void run(StructuredGraph graph) {
-        for (Node node : graph.getNodes().filterInterface(StateSplit.class)) {
-            StateSplit stateSplit = (StateSplit) node;
-            FrameState frameState = stateSplit.stateAfter();
-            if (!stateSplit.hasSideEffect()) {
+        ReentrantNodeIterator.apply(new SnippetFrameStateCleanupClosure(), graph.start(), new CleanupState(false), null);
+    }
+
+    private static class CleanupState {
+
+        public boolean containsFrameState;
+
+        public CleanupState(boolean containsFrameState) {
+            this.containsFrameState = containsFrameState;
+        }
+    }
+
+    /**
+     * A proper (loop-aware) iteration over the graph is used to detect loops that contain invalid
+     * frame states, so that they can be marked with an invalid frame state.
+     */
+    private static class SnippetFrameStateCleanupClosure extends NodeIteratorClosure<CleanupState> {
+
+        @Override
+        protected void processNode(FixedNode node, CleanupState currentState) {
+            if (node instanceof StateSplit) {
+                StateSplit stateSplit = (StateSplit) node;
+                FrameState frameState = stateSplit.stateAfter();
                 if (frameState != null) {
-                    stateSplit.setStateAfter(null);
+                    if (stateSplit.hasSideEffect()) {
+                        currentState.containsFrameState = true;
+                        stateSplit.setStateAfter(node.graph().add(new FrameState(FrameState.INVALID_FRAMESTATE_BCI)));
+                    } else {
+                        stateSplit.setStateAfter(null);
+                    }
                     if (frameState.usages().isEmpty()) {
                         GraphUtil.killWithUnusedFloatingInputs(frameState);
                     }
                 }
             }
         }
+
+        @Override
+        protected CleanupState merge(MergeNode merge, List<CleanupState> states) {
+            for (CleanupState state : states) {
+                if (state.containsFrameState) {
+                    return new CleanupState(true);
+                }
+            }
+            return new CleanupState(false);
+        }
+
+        @Override
+        protected CleanupState afterSplit(BeginNode node, CleanupState oldState) {
+            return new CleanupState(oldState.containsFrameState);
+        }
+
+        @Override
+        protected Map<LoopExitNode, CleanupState> processLoop(LoopBeginNode loop, CleanupState initialState) {
+            LoopInfo<CleanupState> info = ReentrantNodeIterator.processLoop(this, loop, new CleanupState(false));
+            boolean containsFrameState = false;
+            for (CleanupState state : info.endStates.values()) {
+                containsFrameState |= state.containsFrameState;
+            }
+            if (containsFrameState) {
+                loop.setStateAfter(loop.graph().add(new FrameState(FrameState.INVALID_FRAMESTATE_BCI)));
+            }
+            if (containsFrameState || initialState.containsFrameState) {
+                for (CleanupState state : info.exitStates.values()) {
+                    state.containsFrameState = true;
+                }
+            }
+            return info.exitStates;
+        }
+
     }
 }
--- a/graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/SnippetInstaller.java	Wed Feb 27 17:53:21 2013 +0100
+++ b/graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/SnippetInstaller.java	Wed Feb 27 18:28:09 2013 +0100
@@ -89,7 +89,7 @@
                 }
                 ResolvedJavaMethod snippet = runtime.lookupJavaMethod(method);
                 assert snippet.getCompilerStorage().get(Graph.class) == null : method;
-                StructuredGraph graph = makeGraph(snippet, inliningPolicy(snippet), false);
+                StructuredGraph graph = makeGraph(snippet, inliningPolicy(snippet));
                 // System.out.println("snippet: " + graph);
                 snippet.getCompilerStorage().put(Graph.class, graph);
             }
@@ -152,9 +152,8 @@
         substitute = runtime.lookupJavaMethod(substituteMethod);
         original = runtime.lookupJavaMethod(originalMethod);
         try {
-            // System.out.println("substitution: " + MetaUtil.format("%H.%n(%p)", original) +
-            // " --> " + MetaUtil.format("%H.%n(%p)", substitute));
-            StructuredGraph graph = makeGraph(substitute, inliningPolicy(substitute), true);
+            Debug.log("substitution: " + MetaUtil.format("%H.%n(%p)", original) + " --> " + MetaUtil.format("%H.%n(%p)", substitute));
+            StructuredGraph graph = makeGraph(substitute, inliningPolicy(substitute));
             Object oldValue = original.getCompilerStorage().put(Graph.class, graph);
             assert oldValue == null;
         } finally {
@@ -192,7 +191,7 @@
         }
     }
 
-    public StructuredGraph makeGraph(final ResolvedJavaMethod method, final SnippetInliningPolicy policy, final boolean isSubstitution) {
+    public StructuredGraph makeGraph(final ResolvedJavaMethod method, final SnippetInliningPolicy policy) {
         return Debug.scope("BuildSnippetGraph", new Object[]{method}, new Callable<StructuredGraph>() {
 
             @Override
@@ -201,12 +200,8 @@
 
                 new SnippetIntrinsificationPhase(runtime, pool, SnippetTemplate.hasConstantParameter(method)).apply(graph);
 
-                if (isSubstitution && !substituteCallsOriginal) {
-                    // TODO (ds) remove the constraint of only processing substitutions
-                    // once issues with the arraycopy snippets have been resolved
-                    new SnippetFrameStateCleanupPhase().apply(graph);
-                    new DeadCodeEliminationPhase().apply(graph);
-                }
+                new SnippetFrameStateCleanupPhase().apply(graph);
+                new DeadCodeEliminationPhase().apply(graph);
 
                 new InsertStateAfterPlaceholderPhase().apply(graph);
 
@@ -221,7 +216,6 @@
         StructuredGraph graph = graphCache.get(method);
         if (graph == null) {
             graph = buildGraph(method, policy == null ? inliningPolicy(method) : policy);
-            // System.out.println("built " + graph);
             graphCache.put(method, graph);
         }
         return graph;
@@ -248,10 +242,6 @@
                 new GraphBuilderPhase(runtime, GraphBuilderConfiguration.getSnippetDefault(), OptimisticOptimizations.NONE).apply(originalGraph);
                 InliningUtil.inline(invoke, originalGraph, true);
 
-                // TODO the inlined frame states still show the call from the substitute to the
-                // original.
-                // If this poses a problem, a phase should added to fix up these frame states.
-
                 Debug.dump(graph, "after inlining %s", callee);
                 if (GraalOptions.OptCanonicalizer) {
                     new CanonicalizerPhase(runtime, assumptions).apply(graph);