Move validation to validate.dart and add comments.

sdk/lib/_internal/compiler/implementation/ssa/validate.dart

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of ssa;
 
 class HValidator extends HInstructionVisitor {
   bool isValid = true;
   HGraph graph;
 
@@ skipping 18 matching lines @@
     if (block.first == null || block.last == null) {
       markInvalid("empty block");
     }
     if (block.last is !HControlFlow) {
       markInvalid("block ends with non-tail node.");
     }
     if (block.last is HIf && block.successors.length != 2) {
       markInvalid("If node without two successors");
     }
     if (block.last is HConditionalBranch && block.successors.length != 2) {
-      markInvalid("Conditional node without two successors");
+        markInvalid("Conditional node without two successors");
+      }
+    if (block.last is HLoopBranch) {
+      // Assert that the block we inserted to avoid critical edges satisfies
+      // our assumptions. That is, it must not contain any instructions
+      // (although it may contain phi-updates).
+      HBasicBlock avoidCriticalEdgeBlock = block.successors.last;
+      if (avoidCriticalEdgeBlock.first is! HGoto) {
+        markInvalid("Critical edge block contains instructions");
+      }
     }
     if (block.last is HGoto && block.successors.length != 1) {
       markInvalid("Goto node with not exactly one successor");
     }
     if (block.last is HJump && block.successors.length != 1) {
       markInvalid("Break or continue node without one successor");
     }
     if ((block.last is HReturn || block.last is HThrow) &&
         (block.successors.length != 1 || !block.successors[0].isExitBlock())) {
       markInvalid("Return or throw node with > 1 successor "
@@ skipping 10 matching lines @@
     // Check that successors ids are always higher than the current one.
     // TODO(floitsch): this is, of course, not true for back-branches.
     if (block.id == null) markInvalid("block without id");
     for (HBasicBlock successor in block.successors) {
       if (!isValid) break;
       if (successor.id == null) markInvalid("successor without id");
       if (successor.id <= block.id && !successor.isLoopHeader()) {
         markInvalid("successor with lower id, but not a loop-header");
       }
     }
+    // Make sure we don't have a critical edge.
+    if (isValid && block.successors.length > 1 &&
+        block.last is! HTry && block.last is! HExitTry &&
+        block.last is! HSwitch) {

[ngeoffray, 2014/06/25 07:55:22]

Wow, did not realize there was that many special cases :-(

[floitsch, 2014/06/25 08:27:18]

Didn't know either, until all kinds of tests bombed...

+      for (HBasicBlock successor in block.successors) {
+        if (!isValid) break;
+        if (successor.predecessors.length >= 2) {
+          markInvalid("SSA graph contains critical edge.");
+        }
+      }
+    }
 
     // Check that the entries in the dominated-list are sorted.
     int lastId = 0;
     for (HBasicBlock dominated in block.dominatedBlocks) {
       if (!isValid) break;
       if (!identical(dominated.dominator, block)) {
         markInvalid("dominated block not pointing back");
       }
       if (dominated.id == null || dominated.id <= lastId) {
         markInvalid("dominated.id == null or dominated has <= id");
@@ skipping 91 matching lines @@
       markInvalid("Instruction in wrong block");
     }
     if (!hasCorrectInputs()) {
       markInvalid("Incorrect inputs");
     }
     if (!hasCorrectUses()) {
       markInvalid("Incorrect uses");
     }
   }
 }