# HG changeset patch # User Doug Simon # Date 1431360404 -7200 # Node ID ed1fcadffda113cc01318cf62ee39e0dbbbdf0d1 # Parent 3b5ec1a2b3b541cc0c073add2e2ef0f6447711b6 removed FrameStateProcessing and CollapseFrameForSingleSideEffectPhase diff -r 3b5ec1a2b3b5 -r ed1fcadffda1 graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotConstantReflectionProvider.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotConstantReflectionProvider.java Mon May 11 17:12:15 2015 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotConstantReflectionProvider.java Mon May 11 18:06:44 2015 +0200 @@ -35,7 +35,6 @@ import com.oracle.graal.hotspot.*; import com.oracle.graal.options.*; import com.oracle.graal.replacements.*; -import com.oracle.graal.replacements.ReplacementsImpl.FrameStateProcessing; import com.oracle.graal.replacements.SnippetTemplate.Arguments; /** @@ -314,7 +313,7 @@ ResolvedJavaMethod initMethod = null; try { Class rjm = ResolvedJavaMethod.class; - makeGraphMethod = metaAccess.lookupJavaMethod(ReplacementsImpl.class.getDeclaredMethod("makeGraph", rjm, Object[].class, rjm, FrameStateProcessing.class)); + makeGraphMethod = metaAccess.lookupJavaMethod(ReplacementsImpl.class.getDeclaredMethod("makeGraph", rjm, Object[].class, rjm)); initMethod = metaAccess.lookupJavaMethod(SnippetTemplate.AbstractTemplates.class.getDeclaredMethod("template", Arguments.class)); } catch (NoSuchMethodException | SecurityException e) { throw new GraalInternalError(e); diff -r 3b5ec1a2b3b5 -r ed1fcadffda1 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 May 11 17:12:15 2015 +0200 +++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ObjectAccessTest.java Mon May 11 18:06:44 2015 +0200 @@ -32,7 +32,6 @@ import com.oracle.graal.nodes.calc.*; import com.oracle.graal.nodes.extended.*; import com.oracle.graal.replacements.*; -import com.oracle.graal.replacements.ReplacementsImpl.FrameStateProcessing; import com.oracle.graal.word.*; /** @@ -50,7 +49,7 @@ @Override protected StructuredGraph parseEager(ResolvedJavaMethod m, AllowAssumptions allowAssumptions) { - return installer.makeGraph(m, null, null, FrameStateProcessing.CollapseFrameForSingleSideEffect); + return installer.makeGraph(m, null, null); } @Test diff -r 3b5ec1a2b3b5 -r ed1fcadffda1 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 May 11 17:12:15 2015 +0200 +++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PointerTest.java Mon May 11 18:06:44 2015 +0200 @@ -35,7 +35,6 @@ import com.oracle.graal.phases.common.*; import com.oracle.graal.phases.tiers.*; import com.oracle.graal.replacements.*; -import com.oracle.graal.replacements.ReplacementsImpl.FrameStateProcessing; import com.oracle.graal.word.*; import com.oracle.graal.word.nodes.*; @@ -56,7 +55,7 @@ @Override protected StructuredGraph parseEager(ResolvedJavaMethod m, AllowAssumptions allowAssumptions) { - return installer.makeGraph(m, null, null, FrameStateProcessing.CollapseFrameForSingleSideEffect); + return installer.makeGraph(m, null, null); } @Test diff -r 3b5ec1a2b3b5 -r ed1fcadffda1 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 May 11 17:12:15 2015 +0200 +++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/WordTest.java Mon May 11 18:06:44 2015 +0200 @@ -29,7 +29,6 @@ import com.oracle.graal.nodes.*; import com.oracle.graal.nodes.StructuredGraph.AllowAssumptions; import com.oracle.graal.replacements.*; -import com.oracle.graal.replacements.ReplacementsImpl.FrameStateProcessing; import com.oracle.graal.word.*; /** @@ -45,7 +44,7 @@ @Override protected StructuredGraph parseEager(ResolvedJavaMethod m, AllowAssumptions allowAssumptions) { - return installer.makeGraph(m, null, null, FrameStateProcessing.CollapseFrameForSingleSideEffect); + return installer.makeGraph(m, null, null); } @Test diff -r 3b5ec1a2b3b5 -r ed1fcadffda1 graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/CollapseFrameForSingleSideEffectPhase.java --- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/CollapseFrameForSingleSideEffectPhase.java Mon May 11 17:12:15 2015 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,240 +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 static com.oracle.graal.api.code.BytecodeFrame.*; - -import java.util.*; - -import com.oracle.graal.api.code.*; -import com.oracle.graal.graph.*; -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 BytecodeFrame#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 BytecodeFrame#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 { - - private static class IterationState { - public final IterationState previous; - public final Node node; - public final Collection merge; - public final boolean invalid; - - private IterationState(IterationState previous, Node node, Collection merge, boolean invalid) { - this.previous = previous; - this.node = node; - this.merge = merge; - this.invalid = invalid; - } - - public IterationState() { - this(null, null, null, false); - } - - public IterationState addSideEffect(StateSplit sideEffect) { - return new IterationState(this, sideEffect.asNode(), null, true); - } - - public IterationState addBranch(AbstractBeginNode begin) { - return new IterationState(this, begin, null, this.invalid); - } - - public static IterationState merge(AbstractMergeNode merge, Collection before, boolean invalid) { - return new IterationState(null, merge, before, invalid); - } - - public void markAll(NodeBitMap set) { - IterationState state = this; - while (state != null && state.node != null && !set.contains(state.node)) { - set.mark(state.node); - if (state.merge != null) { - for (IterationState branch : state.merge) { - branch.markAll(set); - } - } - state = state.previous; - } - } - - public void markMasked(NodeBitMap unmasked, NodeBitMap masked) { - IterationState state = this; - while (state != null && state.node != null && !masked.contains(state.node)) { - if (state.node instanceof StateSplit) { - unmasked.mark(state.node); - StateSplit split = (StateSplit) state.node; - if (split.hasSideEffect() && state.previous != null) { - state.previous.markAll(masked); - return; - } - } - - if (state.merge != null) { - for (IterationState branch : state.merge) { - branch.markMasked(unmasked, masked); - } - } - state = state.previous; - } - } - } - - @Override - protected void run(StructuredGraph graph) { - CollapseFrameForSingleSideEffectClosure closure = new CollapseFrameForSingleSideEffectClosure(); - ReentrantNodeIterator.apply(closure, graph.start(), new IterationState()); - closure.finishProcessing(graph); - } - - private static class CollapseFrameForSingleSideEffectClosure extends NodeIteratorClosure { - - private List returnStates = new ArrayList<>(); - private List unwindStates = new ArrayList<>(); - - @Override - protected IterationState processNode(FixedNode node, IterationState currentState) { - IterationState state = currentState; - if (node instanceof StateSplit) { - StateSplit stateSplit = (StateSplit) node; - FrameState frameState = stateSplit.stateAfter(); - if (frameState != null) { - if (stateSplit.hasSideEffect()) { - setStateAfter(node.graph(), stateSplit, INVALID_FRAMESTATE_BCI, false); - state = state.addSideEffect(stateSplit); - } else if (currentState.invalid) { - setStateAfter(node.graph(), stateSplit, INVALID_FRAMESTATE_BCI, false); - } else if (stateSplit instanceof StartNode) { - setStateAfter(node.graph(), stateSplit, BEFORE_BCI, false); - } else { - stateSplit.setStateAfter(null); - if (frameState.hasNoUsages()) { - GraphUtil.killWithUnusedFloatingInputs(frameState); - } - } - } - } - if (node instanceof ReturnNode) { - returnStates.add(currentState); - } else if (node instanceof UnwindNode) { - unwindStates.add(currentState); - } - return state; - } - - @Override - protected IterationState merge(AbstractMergeNode merge, List states) { - boolean invalid = false; - for (IterationState state : states) { - if (state.invalid) { - invalid = true; - break; - } - } - return IterationState.merge(merge, states, invalid); - } - - public void finishProcessing(StructuredGraph graph) { - NodeBitMap maskedSideEffects = new NodeBitMap(graph); - NodeBitMap returnSideEffects = new NodeBitMap(graph); - NodeBitMap unwindSideEffects = new NodeBitMap(graph); - - for (IterationState returnState : returnStates) { - returnState.markMasked(returnSideEffects, maskedSideEffects); - } - for (IterationState unwindState : unwindStates) { - unwindState.markMasked(unwindSideEffects, maskedSideEffects); - } - - for (Node returnSideEffect : returnSideEffects) { - if (!unwindSideEffects.contains(returnSideEffect) && !maskedSideEffects.contains(returnSideEffect)) { - StateSplit split = (StateSplit) returnSideEffect; - setStateAfter(graph, split, AFTER_BCI, true); - } - } - - for (Node unwindSideEffect : unwindSideEffects) { - if (!returnSideEffects.contains(unwindSideEffect) && !maskedSideEffects.contains(unwindSideEffect)) { - StateSplit split = (StateSplit) unwindSideEffect; - setStateAfter(graph, split, AFTER_EXCEPTION_BCI, true); - } - } - } - - @Override - protected IterationState afterSplit(AbstractBeginNode node, IterationState oldState) { - return oldState.addBranch(node); - } - - @Override - protected Map processLoop(LoopBeginNode loop, IterationState initialState) { - LoopInfo info = ReentrantNodeIterator.processLoop(this, loop, initialState); - - boolean isNowInvalid = initialState.invalid; - for (IterationState endState : info.endStates.values()) { - isNowInvalid |= endState.invalid; - } - - if (isNowInvalid) { - setStateAfter(loop.graph(), loop, INVALID_FRAMESTATE_BCI, false); - } - - IterationState endState = IterationState.merge(loop, info.endStates.values(), isNowInvalid); - return ReentrantNodeIterator.processLoop(this, loop, endState).exitStates; - } - - /** - * Creates and sets a special frame state for a node. If the existing frame state is - * non-null and has no other usages, it is deleted via - * {@link GraphUtil#killWithUnusedFloatingInputs(Node)}. - * - * @param graph the graph context - * @param node the node whose frame state is updated - * @param bci {@link BytecodeFrame#BEFORE_BCI}, {@link BytecodeFrame#AFTER_EXCEPTION_BCI} or - * {@link BytecodeFrame#INVALID_FRAMESTATE_BCI} - * @param replaceOnly only perform the update if the node currently has a non-null frame - * state - */ - private static void setStateAfter(StructuredGraph graph, StateSplit node, int bci, boolean replaceOnly) { - assert (bci == BEFORE_BCI && node instanceof StartNode) || bci == AFTER_BCI || bci == AFTER_EXCEPTION_BCI || bci == INVALID_FRAMESTATE_BCI; - FrameState currentStateAfter = node.stateAfter(); - if (currentStateAfter != null || !replaceOnly) { - node.setStateAfter(graph.add(new FrameState(bci))); - if (currentStateAfter != null && currentStateAfter.hasNoUsages()) { - GraphUtil.killWithUnusedFloatingInputs(currentStateAfter); - } - } - } - } -} diff -r 3b5ec1a2b3b5 -r ed1fcadffda1 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 May 11 17:12:15 2015 +0200 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java Mon May 11 18:06:44 2015 +0200 @@ -300,9 +300,7 @@ StructuredGraph graph = UseSnippetGraphCache ? graphs.get(method) : null; if (graph == null) { try (DebugCloseable a = SnippetPreparationTime.start()) { - FrameStateProcessing frameStateProcessing = method.getAnnotation(Snippet.class).removeAllFrameStates() ? FrameStateProcessing.Removal - : FrameStateProcessing.CollapseFrameForSingleSideEffect; - StructuredGraph newGraph = makeGraph(method, args, recursiveEntry, frameStateProcessing); + StructuredGraph newGraph = makeGraph(method, args, recursiveEntry); Debug.metric("SnippetNodeCount[%#s]", method).add(newGraph.getNodeCount()); if (!UseSnippetGraphCache || args != null) { return newGraph; @@ -347,7 +345,7 @@ } StructuredGraph graph = graphs.get(substitute); if (graph == null) { - graph = makeGraph(substitute, null, original, FrameStateProcessing.None); + graph = makeGraph(substitute, null, original); graph.freeze(); graphs.putIfAbsent(substitute, graph); graph = graphs.get(substitute); @@ -446,19 +444,18 @@ * @param args * @param original the original method if {@code method} is a {@linkplain MethodSubstitution * substitution} otherwise null - * @param frameStateProcessing controls how {@link FrameState FrameStates} should be handled. */ - public StructuredGraph makeGraph(ResolvedJavaMethod method, Object[] args, ResolvedJavaMethod original, FrameStateProcessing frameStateProcessing) { + public StructuredGraph makeGraph(ResolvedJavaMethod method, Object[] args, ResolvedJavaMethod original) { try (OverrideScope s = OptionValue.override(DeoptALot, false)) { - return createGraphMaker(method, original, frameStateProcessing).makeGraph(args); + return createGraphMaker(method, original).makeGraph(args); } } /** * Can be overridden to return an object that specializes various parts of graph preprocessing. */ - protected GraphMaker createGraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod original, FrameStateProcessing frameStateProcessing) { - return new GraphMaker(this, substitute, original, frameStateProcessing); + protected GraphMaker createGraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod original) { + return new GraphMaker(this, substitute, original); } /** @@ -466,18 +463,6 @@ */ final ConcurrentMap graphCache = new ConcurrentHashMap<>(); - public enum FrameStateProcessing { - None, - /** - * @see CollapseFrameForSingleSideEffectPhase - */ - CollapseFrameForSingleSideEffect, - /** - * Removes frame states from all nodes in the graph. - */ - Removal - } - /** * Creates and preprocesses a graph for a replacement. */ @@ -497,16 +482,10 @@ */ protected final ResolvedJavaMethod substitutedMethod; - /** - * Controls how FrameStates are processed. - */ - private FrameStateProcessing frameStateProcessing; - - protected GraphMaker(ReplacementsImpl replacements, ResolvedJavaMethod substitute, ResolvedJavaMethod substitutedMethod, FrameStateProcessing frameStateProcessing) { + protected GraphMaker(ReplacementsImpl replacements, ResolvedJavaMethod substitute, ResolvedJavaMethod substitutedMethod) { this.replacements = replacements; this.method = substitute; this.substitutedMethod = substitutedMethod; - this.frameStateProcessing = frameStateProcessing; } public StructuredGraph makeGraph(Object[] args) { @@ -537,18 +516,6 @@ new ConvertDeoptimizeToGuardPhase().apply(graph, null); assert sideEffectCount == graph.getNodes().filter(e -> hasSideEffect(e)).count() : "deleted side effecting node"; - switch (frameStateProcessing) { - case Removal: - for (Node node : graph.getNodes()) { - if (node instanceof StateSplit) { - ((StateSplit) node).setStateAfter(null); - } - } - break; - case CollapseFrameForSingleSideEffect: - new CollapseFrameForSingleSideEffectPhase().apply(graph); - break; - } new DeadCodeEliminationPhase(Required).apply(graph); } diff -r 3b5ec1a2b3b5 -r ed1fcadffda1 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 May 11 17:12:15 2015 +0200 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/nodes/MacroNode.java Mon May 11 18:06:44 2015 +0200 @@ -24,7 +24,6 @@ import static com.oracle.graal.api.code.BytecodeFrame.*; -import com.oracle.graal.api.code.*; import com.oracle.graal.api.meta.*; import com.oracle.graal.api.replacements.*; import com.oracle.graal.compiler.common.*; @@ -114,13 +113,6 @@ StructuredGraph methodSubstitution = tool.getReplacements().getSubstitution(getTargetMethod(), true, bci); if (methodSubstitution != null) { methodSubstitution = methodSubstitution.copy(); - if (stateAfter() == null || stateAfter().bci == BytecodeFrame.AFTER_BCI) { - /* - * handles the case of a MacroNode inside a snippet used for another MacroNode - * lowering - */ - new CollapseFrameForSingleSideEffectPhase().apply(methodSubstitution); - } return lowerReplacement(methodSubstitution, tool); } return null;