changeset 15588:9a63ccd66007

[single-pass-iter] additional documentation and assertions
author Miguel Garcia <miguel.m.garcia@oracle.com>
date Sat, 10 May 2014 15:37:51 +0200
parents cad72380191d
children ddb3ef30fcd2
files graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/ConditionalEliminationPhase.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/FlowSensitiveReduction.java graal/com.oracle.graal.phases/src/com/oracle/graal/phases/graph/SinglePassNodeIterator.java
diffstat 3 files changed, 47 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/ConditionalEliminationPhase.java	Fri May 09 20:22:05 2014 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/ConditionalEliminationPhase.java	Sat May 10 15:37:51 2014 +0200
@@ -351,6 +351,7 @@
             if (falseConstant.usages().isEmpty()) {
                 graph.removeFloating(falseConstant);
             }
+            super.finished();
         }
 
         private void registerCondition(boolean isTrue, LogicNode condition, ValueNode anchor) {
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/FlowSensitiveReduction.java	Fri May 09 20:22:05 2014 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/FlowSensitiveReduction.java	Sat May 10 15:37:51 2014 +0200
@@ -165,6 +165,7 @@
         assert !isAliveWithoutUsages(trueConstant);
         assert !isAliveWithoutUsages(falseConstant);
         assert !isAliveWithoutUsages(nullConstant);
+        super.finished();
     }
 
     private static boolean isAliveWithoutUsages(FloatingNode node) {
--- a/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/graph/SinglePassNodeIterator.java	Fri May 09 20:22:05 2014 +0200
+++ b/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/graph/SinglePassNodeIterator.java	Sat May 10 15:37:51 2014 +0200
@@ -56,8 +56,37 @@
 public abstract class SinglePassNodeIterator<T extends MergeableState<T>> {
 
     private final NodeBitMap visitedEnds;
+
+    /**
+     * @see SinglePassNodeIterator.QElem
+     */
     private final Deque<QElem<T>> nodeQueue;
+
+    /**
+     * The keys in this map may be:
+     * <ul>
+     * <li>loop-begins and loop-ends, see {@link #finishLoopEnds(LoopEndNode)}</li>
+     * <li>forward-ends of merge-nodes, see {@link #queueMerge(EndNode)}</li>
+     * </ul>
+     *
+     * <p>
+     * It's tricky to answer whether the state an entry contains is the pre-state or the post-state
+     * for the key in question, because states are mutable. Thus an entry may be created to contain
+     * a pre-state (at the time, as done for a loop-begin in {@link #apply()}) only to make it a
+     * post-state soon after (continuing with the loop-begin example, also in {@link #apply()}). In
+     * any case, given that keys are limited to the nodes mentioned in the previous paragraph, in
+     * all cases an entry can be considered to hold a post-state by the time such entry is
+     * retrieved.
+     * </p>
+     *
+     * <p>
+     * The only method that makes this map grow is {@link #keepForLater(FixedNode, MergeableState)}
+     * and the only one that shrinks it is {@link #pruneEntry(FixedNode)}. To make sure no entry is
+     * left behind inadvertently, asserts in {@link #finished()} are in place.
+     * </p>
+     */
     private final Map<FixedNode, T> nodeStates;
+
     private final StartNode start;
 
     protected T state;
@@ -209,6 +238,14 @@
      * <li>entries in {@link #nodeStates} are pruned for the loop (they aren't going to be looked up
      * again, anyway)</li>
      * </ul>
+     *
+     * <p>
+     * The entries removed by this method were inserted:
+     * <ul>
+     * <li>for the loop-begin, by {@link #apply()}</li>
+     * <li>for loop-ends, by (previous) invocations of this method</li>
+     * </ul>
+     * </p>
      */
     private void finishLoopEnds(LoopEndNode end) {
         assert !visitedEnds.isMarked(end);
@@ -238,8 +275,8 @@
      * {@link #nodeQueue}
      *
      * <p>
-     * {@link #nextQueuedNode()} is in charge of pruning entries for the states of forward-ends
-     * inserted by this method.
+     * {@link #nextQueuedNode()} is in charge of pruning entries (held by {@link #nodeStates}) for
+     * the forward-ends inserted by this method.
      * </p>
      */
     private void queueMerge(EndNode end) {
@@ -287,6 +324,11 @@
 
     /**
      * The lifecycle that single-pass node iterators go through is described in {@link #apply()}
+     *
+     * <p>
+     * When overriding this method don't forget to invoke this implementation, otherwise the
+     * assertions will be skipped.
+     * </p>
      */
     protected void finished() {
         assert nodeQueue.isEmpty();
@@ -295,6 +337,7 @@
 
     private void keepForLater(FixedNode x, T s) {
         assert !nodeStates.containsKey(x);
+        assert (x instanceof LoopBeginNode) || (x instanceof LoopEndNode) || (x instanceof EndNode);
         nodeStates.put(x, s);
     }