changeset 13618:b87fcab6624a

Replacements: use enum to describe framestate action
author Bernhard Urban <bernhard.urban@jku.at>
date Mon, 13 Jan 2014 19:48:35 +0200
parents f9ee4532da8f
children 5a076e52220a
files graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/stubs/GraphKit.java graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ObjectAccessTest.java graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PointerTest.java graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/WordTest.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/CollapseFrameForSingleSideEffectPhase.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetFrameStateCleanupPhase.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/nodes/MacroNode.java
diffstat 8 files changed, 161 insertions(+), 153 deletions(-) [+]
line wrap: on
line diff
--- 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);
     }
 }
--- 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
--- 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
--- 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
--- /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<StateSplit> {
+
+        @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<StateSplit> 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<LoopExitNode, StateSplit> processLoop(LoopBeginNode loop, StateSplit initialState) {
+            LoopInfo<StateSplit> 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));
+        }
+    }
+}
--- 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<ResolvedJavaMethod, StructuredGraph> 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);
         }
--- 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<StateSplit> {
-
-        @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<StateSplit> 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<LoopExitNode, StateSplit> processLoop(LoopBeginNode loop, StateSplit initialState) {
-            LoopInfo<StateSplit> 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));
-        }
-    }
-}
--- 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);
         }