# HG changeset patch
# User Christos Kotselidis <christos.kotselidis@oracle.com>
# Date 1366885538 -7200
# Node ID 25c2e9b29e970ff05357046f7e198e1a57c13057
# Parent  9a30199f7ff6f35fc55160875a58f2705c060b73
Add comments to write barrier verification phase

diff -r 9a30199f7ff6 -r 25c2e9b29e97 graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/WriteBarrierVerificationPhase.java
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/WriteBarrierVerificationPhase.java	Thu Apr 25 12:03:27 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/WriteBarrierVerificationPhase.java	Thu Apr 25 12:25:38 2013 +0200
@@ -25,6 +25,7 @@
 
 import java.util.*;
 
+import com.oracle.graal.api.code.*;
 import com.oracle.graal.graph.*;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.nodes.extended.*;
@@ -32,6 +33,13 @@
 import com.oracle.graal.nodes.java.*;
 import com.oracle.graal.phases.*;
 
+/**
+ * Verification phase that checks if, for every write, at least one write barrier is present between
+ * safepoint intervals. Initially, the algorithm performs a bottom-up traversal of the graph in
+ * order to discover safepoints. For every safepoint discovered, a brute-force bottom-up traversal
+ * is performed again that validates every write until all possible reachable safepoints from the
+ * currently processed write.
+ */
 public class WriteBarrierVerificationPhase extends Phase {
 
     @Override
@@ -48,11 +56,18 @@
     }
 
     private static void verifyWrites(Node safepoint) {
+        /*
+         * The list below holds all already processed writes in order to avoid repetitive
+         * validations of writes reachable from different paths.
+         */
+        List<Node> processedWrites = new LinkedList<>();
         Deque<Node> frontier = new ArrayDeque<>();
-        List<Node> processedWrites = new LinkedList<>();
         expandFrontier(frontier, safepoint);
         while (!frontier.isEmpty()) {
             Node currentNode = frontier.removeFirst();
+            /*
+             * Any safepoints reached at this point are skipped since they will be processed later.
+             */
             if (isSafepoint(currentNode)) {
                 continue;
             }
@@ -62,11 +77,14 @@
             }
             expandFrontier(frontier, currentNode);
         }
-
     }
 
     private static void validateWrite(Node write) {
-        if (hasCorrectAttachedBarrier(write)) {
+        /*
+         * The currently validated write is checked in order to discover if it has an appropriate
+         * attached write barrier.
+         */
+        if (hasAttachedBarrier(write)) {
             return;
         }
         Deque<Node> frontier = new ArrayDeque<>();
@@ -74,14 +92,14 @@
         while (!frontier.isEmpty()) {
             Node currentNode = frontier.removeFirst();
             assert !isSafepoint(currentNode) : "Write barrier must be present";
-            if (!(currentNode instanceof SerialWriteBarrier) || ((currentNode instanceof SerialWriteBarrier) && !foundCorrectBarrier(write, currentNode))) {
+            if (!(currentNode instanceof SerialWriteBarrier) || ((currentNode instanceof SerialWriteBarrier) && !validateBarrier(write, currentNode))) {
                 expandFrontier(frontier, currentNode);
             }
         }
     }
 
-    private static boolean hasCorrectAttachedBarrier(Node node) {
-        return (((FixedWithNextNode) node).next() instanceof SerialWriteBarrier) && foundCorrectBarrier(node, ((FixedWithNextNode) node).next());
+    private static boolean hasAttachedBarrier(Node node) {
+        return (((FixedWithNextNode) node).next() instanceof SerialWriteBarrier) && validateBarrier(node, ((FixedWithNextNode) node).next());
     }
 
     private static boolean isObjectWrite(Node node) {
@@ -95,7 +113,8 @@
     private static void expandFrontier(Deque<Node> frontier, Node node) {
         for (Node previousNode : node.cfgPredecessors()) {
             if (previousNode != null) {
-                if (previousNode instanceof ControlSplitNode && frontier.contains(previousNode)) {
+                // Control split nodes are processed only once.
+                if ((previousNode instanceof ControlSplitNode) && frontier.contains(previousNode)) {
                     continue;
                 }
                 frontier.addFirst(previousNode);
@@ -107,7 +126,7 @@
         return ((node instanceof DeoptimizingNode) && ((DeoptimizingNode) node).canDeoptimize()) || (node instanceof LoopBeginNode);
     }
 
-    private static boolean foundCorrectBarrier(Node write, Node barrier) {
+    private static boolean validateBarrier(Node write, Node barrier) {
         SerialWriteBarrier barrierNode = (SerialWriteBarrier) barrier;
         if (write instanceof WriteNode) {
             WriteNode writeNode = (WriteNode) write;