# HG changeset patch # User Bernhard Urban # Date 1389635315 -7200 # Node ID b87fcab6624aa606f5e56c8c7426b95d8e5c1d49 # Parent f9ee4532da8f86df8b7512b8fcddb81995a88d95 Replacements: use enum to describe framestate action diff -r f9ee4532da8f -r b87fcab6624a graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/stubs/GraphKit.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/stubs/GraphKit.java Mon Jan 13 18:37:27 2014 +0100 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/stubs/GraphKit.java Mon Jan 13 19:48:35 2014 +0200 @@ -35,6 +35,7 @@ import com.oracle.graal.phases.common.*; import com.oracle.graal.phases.util.*; import com.oracle.graal.replacements.*; +import com.oracle.graal.replacements.ReplacementsImpl.*; import com.oracle.graal.word.phases.*; /** @@ -158,7 +159,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, method, null, false, false); + StructuredGraph calleeGraph = repl.makeGraph(method, null, method, null, FrameStateProcessing.CollapseFrameForSingleSideEffect); InliningUtil.inline(invoke, calleeGraph, false); } } diff -r f9ee4532da8f -r b87fcab6624a graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ObjectAccessTest.java --- a/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ObjectAccessTest.java Mon Jan 13 18:37:27 2014 +0100 +++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ObjectAccessTest.java Mon Jan 13 19:48:35 2014 +0200 @@ -33,6 +33,7 @@ import com.oracle.graal.nodes.calc.*; import com.oracle.graal.nodes.extended.*; import com.oracle.graal.replacements.*; +import com.oracle.graal.replacements.ReplacementsImpl.*; import com.oracle.graal.replacements.Snippet.SnippetInliningPolicy; import com.oracle.graal.word.*; @@ -54,7 +55,7 @@ @Override protected StructuredGraph parse(Method m) { ResolvedJavaMethod resolvedMethod = getMetaAccess().lookupJavaMethod(m); - return installer.makeGraph(resolvedMethod, null, resolvedMethod, inliningPolicy.get(), false, false); + return installer.makeGraph(resolvedMethod, null, resolvedMethod, inliningPolicy.get(), FrameStateProcessing.CollapseFrameForSingleSideEffect); } @Test diff -r f9ee4532da8f -r b87fcab6624a graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PointerTest.java --- a/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PointerTest.java Mon Jan 13 18:37:27 2014 +0100 +++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PointerTest.java Mon Jan 13 19:48:35 2014 +0200 @@ -36,6 +36,7 @@ import com.oracle.graal.phases.common.*; import com.oracle.graal.phases.tiers.*; import com.oracle.graal.replacements.*; +import com.oracle.graal.replacements.ReplacementsImpl.*; import com.oracle.graal.replacements.Snippet.SnippetInliningPolicy; import com.oracle.graal.word.*; import com.oracle.graal.word.nodes.*; @@ -60,7 +61,7 @@ @Override protected StructuredGraph parse(Method m) { ResolvedJavaMethod resolvedMethod = getMetaAccess().lookupJavaMethod(m); - return installer.makeGraph(resolvedMethod, null, resolvedMethod, inliningPolicy.get(), false, false); + return installer.makeGraph(resolvedMethod, null, resolvedMethod, inliningPolicy.get(), FrameStateProcessing.CollapseFrameForSingleSideEffect); } @Test diff -r f9ee4532da8f -r b87fcab6624a graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/WordTest.java --- a/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/WordTest.java Mon Jan 13 18:37:27 2014 +0100 +++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/WordTest.java Mon Jan 13 19:48:35 2014 +0200 @@ -29,6 +29,7 @@ import com.oracle.graal.compiler.test.*; import com.oracle.graal.nodes.*; import com.oracle.graal.replacements.*; +import com.oracle.graal.replacements.ReplacementsImpl.*; import com.oracle.graal.replacements.Snippet.SnippetInliningPolicy; import com.oracle.graal.test.*; import com.oracle.graal.word.*; @@ -49,7 +50,7 @@ @Override protected StructuredGraph parse(Method m) { ResolvedJavaMethod resolvedMethod = getMetaAccess().lookupJavaMethod(m); - return installer.makeGraph(resolvedMethod, null, resolvedMethod, inliningPolicy.get(), false, false); + return installer.makeGraph(resolvedMethod, null, resolvedMethod, inliningPolicy.get(), FrameStateProcessing.CollapseFrameForSingleSideEffect); } @LongTest diff -r f9ee4532da8f -r b87fcab6624a graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/CollapseFrameForSingleSideEffectPhase.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/CollapseFrameForSingleSideEffectPhase.java Mon Jan 13 19:48:35 2014 +0200 @@ -0,0 +1,126 @@ +/* + * 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.replacements; + +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; + +/** + * This phase ensures that there's a single {@linkplain FrameState#AFTER_BCI collapsed frame state} + * per path. + * + * Removes other frame states from {@linkplain StateSplit#hasSideEffect() non-side-effecting} nodes + * in the graph, and replaces them with {@linkplain FrameState#INVALID_FRAMESTATE_BCI invalid frame + * states}. + * + * The invalid frame states ensure that no deoptimization to a snippet frame state will happen. + */ +public class CollapseFrameForSingleSideEffectPhase extends Phase { + @Override + protected void run(StructuredGraph graph) { + ReentrantNodeIterator.apply(new CollapseFrameForSingleSideEffectClosure(), graph.start(), null, null); + } + + private static class CollapseFrameForSingleSideEffectClosure extends NodeIteratorClosure { + + @Override + protected StateSplit processNode(FixedNode node, StateSplit currentState) { + StateSplit state = currentState; + if (node instanceof StateSplit) { + StateSplit stateSplit = (StateSplit) node; + FrameState frameState = stateSplit.stateAfter(); + if (frameState != null) { + // the stateSplit == currentState case comes from merge handling + if (stateSplit.hasSideEffect() || stateSplit == currentState) { + stateSplit.setStateAfter(createInvalidFrameState(node)); + state = stateSplit; + } else if (hasInvalidState(state)) { + stateSplit.setStateAfter(createInvalidFrameState(node)); + } else { + stateSplit.setStateAfter(null); + } + if (frameState.usages().isEmpty()) { + GraphUtil.killWithUnusedFloatingInputs(frameState); + } + } + } + if (node instanceof ControlSinkNode && state != null) { + state.setStateAfter(node.graph().add(new FrameState(FrameState.AFTER_BCI))); + } + return state; + } + + @Override + protected StateSplit merge(MergeNode merge, List states) { + boolean invalid = false; + for (StateSplit state : states) { + if (state != null && state.stateAfter() != null && state.stateAfter().bci == FrameState.INVALID_FRAMESTATE_BCI) { + invalid = true; + state.setStateAfter(merge.graph().add(new FrameState(FrameState.AFTER_BCI))); + } + } + if (invalid) { + // at the next processNode call, stateSplit == currentState == merge + return merge; + } else { + return null; + } + } + + @Override + protected StateSplit afterSplit(AbstractBeginNode node, StateSplit oldState) { + return oldState; + } + + @Override + protected Map processLoop(LoopBeginNode loop, StateSplit initialState) { + LoopInfo info = ReentrantNodeIterator.processLoop(this, loop, initialState); + if (!hasInvalidState(initialState)) { + boolean isNowInvalid = false; + for (StateSplit endState : info.endStates.values()) { + isNowInvalid |= hasInvalidState(endState); + } + if (isNowInvalid) { + loop.setStateAfter(createInvalidFrameState(loop)); + info = ReentrantNodeIterator.processLoop(this, loop, loop); + } + } + return info.exitStates; + } + + private static boolean hasInvalidState(StateSplit state) { + assert state == null || (state.stateAfter() != null && state.stateAfter().bci == FrameState.INVALID_FRAMESTATE_BCI) : state + " " + state.stateAfter(); + return state != null; + } + + private static FrameState createInvalidFrameState(FixedNode node) { + return node.graph().add(new FrameState(FrameState.INVALID_FRAMESTATE_BCI)); + } + } +} diff -r f9ee4532da8f -r b87fcab6624a graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java --- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java Mon Jan 13 18:37:27 2014 +0100 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java Mon Jan 13 19:48:35 2014 +0200 @@ -96,7 +96,9 @@ StructuredGraph graph = UseSnippetGraphCache ? graphs.get(method) : null; if (graph == null) { try (TimerCloseable a = SnippetPreparationTime.start()) { - StructuredGraph newGraph = makeGraph(method, recursiveEntry, recursiveEntry, inliningPolicy(method), method.getAnnotation(Snippet.class).removeAllFrameStates(), false); + FrameStateProcessing frameStateProcessing = method.getAnnotation(Snippet.class).removeAllFrameStates() ? FrameStateProcessing.Removal + : FrameStateProcessing.CollapseFrameForSingleSideEffect; + StructuredGraph newGraph = makeGraph(method, recursiveEntry, recursiveEntry, inliningPolicy(method), frameStateProcessing); Debug.metric("SnippetNodeCount[" + method.getName() + "]").add(newGraph.getNodeCount()); if (!UseSnippetGraphCache) { return newGraph; @@ -131,7 +133,7 @@ } StructuredGraph graph = graphs.get(substitute); if (graph == null) { - graphs.putIfAbsent(substitute, makeGraph(substitute, original, substitute, inliningPolicy(substitute), false, true)); + graphs.putIfAbsent(substitute, makeGraph(substitute, original, substitute, inliningPolicy(substitute), FrameStateProcessing.None)); graph = graphs.get(substitute); } return graph; @@ -261,18 +263,17 @@ * @param original the original method if {@code method} is a {@linkplain MethodSubstitution * substitution} otherwise null * @param policy the inlining policy to use during preprocessing - * @param removeAllFrameStates removes all frame states from side effecting instructions + * @param frameStateProcessing controls how {@link FrameState FrameStates} should be handled. */ - public StructuredGraph makeGraph(ResolvedJavaMethod method, ResolvedJavaMethod original, ResolvedJavaMethod recursiveEntry, SnippetInliningPolicy policy, boolean removeAllFrameStates, - boolean isMethodSubstitution) { - return createGraphMaker(method, original, recursiveEntry, isMethodSubstitution).makeGraph(policy, removeAllFrameStates); + public StructuredGraph makeGraph(ResolvedJavaMethod method, ResolvedJavaMethod original, ResolvedJavaMethod recursiveEntry, SnippetInliningPolicy policy, FrameStateProcessing frameStateProcessing) { + return createGraphMaker(method, original, recursiveEntry, frameStateProcessing).makeGraph(policy); } /** * Can be overridden to return an object that specializes various parts of graph preprocessing. */ - protected GraphMaker createGraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod original, ResolvedJavaMethod recursiveEntry, boolean isMethodSubstitution) { - return new GraphMaker(substitute, original, recursiveEntry, isMethodSubstitution); + protected GraphMaker createGraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod original, ResolvedJavaMethod recursiveEntry, FrameStateProcessing frameStateProcessing) { + return new GraphMaker(substitute, original, recursiveEntry, frameStateProcessing); } /** @@ -280,6 +281,10 @@ */ final ConcurrentMap graphCache = new ConcurrentHashMap<>(); + public enum FrameStateProcessing { + None, CollapseFrameForSingleSideEffect, Removal + } + /** * Creates and preprocesses a graph for a replacement. */ @@ -300,26 +305,25 @@ protected final ResolvedJavaMethod recursiveEntry; /** - * Controls if FrameStates should be removed or processed with the - * {@link SnippetFrameStateCleanupPhase}. + * Controls how FrameStates are processed. */ - protected final boolean isMethodSubstitution; + private FrameStateProcessing frameStateProcessing; - protected GraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod substitutedMethod, ResolvedJavaMethod recursiveEntry, boolean isMethodSubstitution) { + protected GraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod substitutedMethod, ResolvedJavaMethod recursiveEntry, FrameStateProcessing frameStateProcessing) { this.method = substitute; this.substitutedMethod = substitutedMethod; this.recursiveEntry = recursiveEntry; - this.isMethodSubstitution = isMethodSubstitution; + this.frameStateProcessing = frameStateProcessing; } - public StructuredGraph makeGraph(final SnippetInliningPolicy policy, final boolean removeAllFrameStates) { + public StructuredGraph makeGraph(final SnippetInliningPolicy policy) { try (Scope s = Debug.scope("BuildSnippetGraph", method)) { StructuredGraph graph = parseGraph(method, policy); // Cannot have a finalized version of a graph in the cache graph = graph.copy(); - finalizeGraph(graph, removeAllFrameStates); + finalizeGraph(graph); Debug.dump(graph, "%s: Final", method.getName()); @@ -332,23 +336,24 @@ /** * Does final processing of a snippet graph. */ - protected void finalizeGraph(StructuredGraph graph, boolean removeAllFrameStates) { + protected void finalizeGraph(StructuredGraph graph) { new NodeIntrinsificationPhase(providers).apply(graph); if (!SnippetTemplate.hasConstantParameter(method)) { NodeIntrinsificationVerificationPhase.verify(graph); } new ConvertDeoptimizeToGuardPhase().apply(graph); - if (!isMethodSubstitution) { - if (removeAllFrameStates) { + switch (frameStateProcessing) { + case Removal: for (Node node : graph.getNodes()) { if (node instanceof StateSplit) { ((StateSplit) node).setStateAfter(null); } } - } else { - new SnippetFrameStateCleanupPhase().apply(graph); - } + break; + case CollapseFrameForSingleSideEffect: + new CollapseFrameForSingleSideEffectPhase().apply(graph); + break; } new DeadCodeEliminationPhase().apply(graph); } diff -r f9ee4532da8f -r b87fcab6624a graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetFrameStateCleanupPhase.java --- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetFrameStateCleanupPhase.java Mon Jan 13 18:37:27 2014 +0100 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,127 +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.graal.replacements; - -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) { - ReentrantNodeIterator.apply(new SnippetFrameStateCleanupClosure(), graph.start(), null, null); - } - - private static class SnippetFrameStateCleanupClosure extends NodeIteratorClosure { - - @Override - protected StateSplit processNode(FixedNode node, StateSplit currentState) { - StateSplit state = currentState; - if (node instanceof StateSplit) { - StateSplit stateSplit = (StateSplit) node; - FrameState frameState = stateSplit.stateAfter(); - if (frameState != null) { - // the stateSplit == currentState case comes from merge handling - if (stateSplit.hasSideEffect() || stateSplit == currentState) { - stateSplit.setStateAfter(createInvalidFrameState(node)); - state = stateSplit; - } else if (hasInvalidState(state)) { - stateSplit.setStateAfter(createInvalidFrameState(node)); - } else { - stateSplit.setStateAfter(null); - } - if (frameState.usages().isEmpty()) { - GraphUtil.killWithUnusedFloatingInputs(frameState); - } - } - } - if (node instanceof ControlSinkNode && state != null) { - state.setStateAfter(node.graph().add(new FrameState(FrameState.AFTER_BCI))); - } - return state; - } - - @Override - protected StateSplit merge(MergeNode merge, List states) { - boolean invalid = false; - for (StateSplit state : states) { - if (state != null && state.stateAfter() != null && state.stateAfter().bci == FrameState.INVALID_FRAMESTATE_BCI) { - invalid = true; - state.setStateAfter(merge.graph().add(new FrameState(FrameState.AFTER_BCI))); - } - } - if (invalid) { - // at the next processNode call, stateSplit == currentState == merge - return merge; - } else { - return null; - } - } - - @Override - protected StateSplit afterSplit(AbstractBeginNode node, StateSplit oldState) { - return oldState; - } - - @Override - protected Map processLoop(LoopBeginNode loop, StateSplit initialState) { - LoopInfo info = ReentrantNodeIterator.processLoop(this, loop, initialState); - if (!hasInvalidState(initialState)) { - boolean isNowInvalid = false; - for (StateSplit endState : info.endStates.values()) { - isNowInvalid |= hasInvalidState(endState); - } - if (isNowInvalid) { - loop.setStateAfter(createInvalidFrameState(loop)); - info = ReentrantNodeIterator.processLoop(this, loop, loop); - } - } - return info.exitStates; - } - - private static boolean hasInvalidState(StateSplit state) { - assert state == null || (state.stateAfter() != null && state.stateAfter().bci == FrameState.INVALID_FRAMESTATE_BCI) : state + " " + state.stateAfter(); - return state != null; - } - - private static FrameState createInvalidFrameState(FixedNode node) { - return node.graph().add(new FrameState(FrameState.INVALID_FRAMESTATE_BCI)); - } - } -} diff -r f9ee4532da8f -r b87fcab6624a graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/nodes/MacroNode.java --- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/nodes/MacroNode.java Mon Jan 13 18:37:27 2014 +0100 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/nodes/MacroNode.java Mon Jan 13 19:48:35 2014 +0200 @@ -92,7 +92,7 @@ * handles the case of a MacroNode inside a snippet used for another MacroNode * lowering */ - new SnippetFrameStateCleanupPhase().apply(methodSubstitution); + new CollapseFrameForSingleSideEffectPhase().apply(methodSubstitution); } return lowerReplacement(methodSubstitution.copy(), tool); }