# HG changeset patch # User Gilles Duboscq # Date 1379433911 -7200 # Node ID 8a3b59397044d196ea93253a9856be6972331db4 # Parent bffe5758c20904a74fd6940df58c8875b2bd43aa The SnippetFrameStateCleanupPhase now sets invalid framestates on the paths of side effecting instruction except for the last one where an AFTER_BCI is used. Remove InsertStateAfterPlaceholderPhase diff -r bffe5758c209 -r 8a3b59397044 graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InsertStateAfterPlaceholderPhase.java --- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InsertStateAfterPlaceholderPhase.java Tue Sep 17 17:01:39 2013 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,83 +0,0 @@ -/* - * Copyright (c) 2012, 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.phases.common; - -import com.oracle.graal.graph.*; -import com.oracle.graal.nodes.*; -import com.oracle.graal.nodes.spi.*; -import com.oracle.graal.nodes.type.*; -import com.oracle.graal.phases.*; - -public class InsertStateAfterPlaceholderPhase extends Phase { - - private static class PlaceholderNode extends AbstractStateSplit implements StateSplit, IterableNodeType, LIRLowerable, Canonicalizable { - - public PlaceholderNode() { - super(StampFactory.forVoid()); - } - - @Override - public void generate(LIRGeneratorTool gen) { - // nothing to do - } - - @Override - public boolean hasSideEffect() { - return false; - } - - @Override - public ValueNode canonical(CanonicalizerTool tool) { - if (!usages().isEmpty()) { - return this; - } - if (stateAfter() == null) { - return null; - } - FixedNode next = next(); - if (next instanceof PlaceholderNode && ((PlaceholderNode) next).stateAfter() != null) { - return null; - } - return this; - } - } - - @Override - protected void run(StructuredGraph graph) { - boolean needsPlaceHolder = false; - for (Node node : graph.getNodes().filterInterface(StateSplit.class)) { - StateSplit stateSplit = (StateSplit) node; - if (stateSplit.hasSideEffect() && stateSplit.stateAfter() != null) { - needsPlaceHolder = true; - } - } - if (needsPlaceHolder) { - for (ReturnNode ret : graph.getNodes().filter(ReturnNode.class)) { - PlaceholderNode p = graph.add(new PlaceholderNode()); - p.setStateAfter(graph.add(new FrameState(FrameState.AFTER_BCI))); - graph.addBeforeFixed(ret, p); - } - } - } - -} diff -r bffe5758c209 -r 8a3b59397044 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 Tue Sep 17 17:01:39 2013 +0200 +++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PointerTest.java Tue Sep 17 18:05:11 2013 +0200 @@ -131,7 +131,7 @@ WriteNode write = (WriteNode) graph.start().next(); Assert.assertEquals(graph.getLocal(2), write.value()); Assert.assertEquals(Kind.Void, write.kind()); - Assert.assertEquals(FrameState.INVALID_FRAMESTATE_BCI, write.stateAfter().bci); + Assert.assertEquals(FrameState.AFTER_BCI, write.stateAfter().bci); UnsafeCastNode cast = (UnsafeCastNode) write.object(); Assert.assertEquals(graph.getLocal(0), cast.object()); @@ -150,10 +150,7 @@ Assert.assertEquals(graph.getLocal(1), location.getIndex()); } - AbstractStateSplit stateSplit = (AbstractStateSplit) write.next(); - Assert.assertEquals(FrameState.AFTER_BCI, stateSplit.stateAfter().bci); - - ReturnNode ret = (ReturnNode) stateSplit.next(); + ReturnNode ret = (ReturnNode) write.next(); Assert.assertEquals(null, ret.result()); } diff -r bffe5758c209 -r 8a3b59397044 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 Tue Sep 17 17:01:39 2013 +0200 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java Tue Sep 17 18:05:11 2013 +0200 @@ -292,11 +292,8 @@ if (original == null) { new SnippetFrameStateCleanupPhase().apply(graph); - new DeadCodeEliminationPhase().apply(graph); - new InsertStateAfterPlaceholderPhase().apply(graph); - } else { - new DeadCodeEliminationPhase().apply(graph); } + new DeadCodeEliminationPhase().apply(graph); } private StructuredGraph parseGraph(final ResolvedJavaMethod methodToParse, final SnippetInliningPolicy policy) { diff -r bffe5758c209 -r 8a3b59397044 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 Tue Sep 17 17:01:39 2013 +0200 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetFrameStateCleanupPhase.java Tue Sep 17 18:05:11 2013 +0200 @@ -45,24 +45,23 @@ @Override protected void run(StructuredGraph graph) { - ReentrantNodeIterator.apply(new SnippetFrameStateCleanupClosure(), graph.start(), false, null); + ReentrantNodeIterator.apply(new SnippetFrameStateCleanupClosure(), graph.start(), null, null); } - /** - * 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 { + private static class SnippetFrameStateCleanupClosure extends NodeIteratorClosure { @Override - protected Boolean processNode(FixedNode node, Boolean currentState) { + 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) { if (stateSplit.hasSideEffect()) { - stateSplit.setStateAfter(node.graph().add(new FrameState(FrameState.INVALID_FRAMESTATE_BCI))); - return true; + stateSplit.setStateAfter(createInvalidFrameState(node)); + state = stateSplit; + } else if (hasInvalidState(state)) { + stateSplit.setStateAfter(createInvalidFrameState(node)); } else { stateSplit.setStateAfter(null); } @@ -71,41 +70,44 @@ } } } - return currentState; + if (node instanceof ControlSinkNode && state != null) { + state.setStateAfter(node.graph().add(new FrameState(FrameState.AFTER_BCI))); + } + return state; } @Override - protected Boolean merge(MergeNode merge, List states) { - for (boolean state : states) { - if (state) { - return true; - } - } - return false; + protected StateSplit merge(MergeNode merge, List states) { + return merge; } @Override - protected Boolean afterSplit(AbstractBeginNode node, Boolean oldState) { + protected StateSplit afterSplit(AbstractBeginNode node, StateSplit oldState) { return oldState; } @Override - protected Map processLoop(LoopBeginNode loop, Boolean initialState) { - LoopInfo info = ReentrantNodeIterator.processLoop(this, loop, false); - boolean containsFrameState = false; - for (Boolean state : info.endStates.values()) { - containsFrameState |= state; - } - if (containsFrameState) { - loop.setStateAfter(loop.graph().add(new FrameState(FrameState.INVALID_FRAMESTATE_BCI))); - } - if (containsFrameState || initialState) { - for (Map.Entry entry : info.exitStates.entrySet()) { - entry.setValue(true); + 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) { + return state != null && state.stateAfter() != null && state.stateAfter().bci == FrameState.INVALID_FRAMESTATE_BCI; + } + + private static FrameState createInvalidFrameState(FixedNode node) { + return node.graph().add(new FrameState(FrameState.INVALID_FRAMESTATE_BCI)); + } } }