# HG changeset patch # User Lukas Stadler # Date 1361986089 -3600 # Node ID 93a133fc03ce5fb0651b1780fee63435a6bac55e # Parent 30d754a0e87cb775237e180b105fb90cc32e91d0 clear frame states in snippets and replace with sentries diff -r 30d754a0e87c -r 93a133fc03ce graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/FrameState.java --- 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 values; diff -r 30d754a0e87c -r 93a133fc03ce graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningUtil.java --- 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); diff -r 30d754a0e87c -r 93a133fc03ce graal/com.oracle.graal.snippets.test/src/com/oracle/graal/snippets/WordTest.java --- 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 diff -r 30d754a0e87c -r 93a133fc03ce graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/SnippetFrameStateCleanupPhase.java --- 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 { + + @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 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 processLoop(LoopBeginNode loop, CleanupState initialState) { + LoopInfo 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; + } + } } diff -r 30d754a0e87c -r 93a133fc03ce graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/SnippetInstaller.java --- 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() { @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);