Fix crash in compiler when we are generating a bailout method where a loop contains continue/break/…

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

Index: sdk/lib/_internal/compiler/implementation/ssa/codegen.dart
===================================================================
--- sdk/lib/_internal/compiler/implementation/ssa/codegen.dart	(revision 17773)
+++ sdk/lib/_internal/compiler/implementation/ssa/codegen.dart	(working copy)
@@ -337,10 +337,6 @@
   beginGraph(HGraph graph);
   endGraph(HGraph graph);
 
-  beginLoop(HBasicBlock block);
-  endLoop(HBasicBlock block);
-  handleLoopCondition(HLoopBranch node);
-
   preLabeledBlock(HLabeledBlockInformation labeledBlockInfo);
   startLabeledBlock(HLabeledBlockInformation labeledBlockInfo);
   endLabeledBlock(HLabeledBlockInformation labeledBlockInfo);
@@ -959,11 +955,7 @@
         assignPhisOfSuccessors(avoidEdge);
         bool hasPhiUpdates = !updateBody.statements.isEmpty;
         currentContainer = body;
-        if (hasPhiUpdates || !isConditionExpression || info.updates != null) {
-          wrapLoopBodyForContinue(info);
-        } else {
-          visitBodyIgnoreLabels(info);
-        }
+        visitBodyIgnoreLabels(info);
         if (info.updates != null) {
           generateStatements(info.updates);
         }
@@ -1125,15 +1117,9 @@
     currentBlock = node;
     // If this node has block-structure based information attached,
     // try using that to traverse from here.
-    if (node.blockFlow != null &&
-        handleBlockFlow(node.blockFlow)) {
+    if (node.blockFlow != null && handleBlockFlow(node.blockFlow)) {
       return;
     }
-    // Flow based traversal.
-    if (node.isLoopHeader() &&
-        !identical(node.loopInformation.loopBlockInformation, currentBlockInformation)) {
-      beginLoop(node);
-    }
     iterateBasicBlock(node);
   }
 
@@ -1833,43 +1819,6 @@
     world.registerInstantiatedClass(type.element);
   }
 
-  visitLoopBranch(HLoopBranch node) {
-    if (subGraph != null && identical(node.block, subGraph.end)) {
-      // We are generating code for a loop condition.
-      // If doing this as part of a SubGraph traversal, the
-      // calling code will handle the control flow logic.
-
-      // If we are generating the subgraph as an expression, the
-      // condition will be generated as the expression.
-      // Otherwise, we don't generate the expression, and leave that
-      // to the code that called [visitSubGraph].
-      if (isGeneratingExpression) {
-        use(node.inputs[0]);
-      }
-      return;
-    }
-    HBasicBlock branchBlock = currentBlock;
-    handleLoopCondition(node);
-    List<HBasicBlock> dominated = currentBlock.dominatedBlocks;
-    if (!node.isDoWhile()) {
-      // For a do while loop, the body has already been visited.
-      visitBasicBlock(dominated[0]);
-    }
-    endLoop(node.block);
-
-    // If the branch does not dominate the code after the loop, the
-    // dominator will visit it.
-    if (!identical(branchBlock.successors[1].dominator, branchBlock)) return;
-
-    visitBasicBlock(branchBlock.successors[1]);
-    // With labeled breaks we can have more dominated blocks.
-    if (dominated.length >= 3) {
-      for (int i = 2; i < dominated.length; i++) {
-        visitBasicBlock(dominated[i]);
-      }
-    }
-  }
-
   visitNot(HNot node) {
     assert(node.inputs.length == 1);
     generateNot(node.inputs[0]);
@@ -2655,33 +2604,25 @@
     // Do nothing. Bailout targets are only used in the non-optimized version.
   }
 
-  void beginLoop(HBasicBlock block) {
-    oldContainerStack.add(currentContainer);
-    currentContainer = new js.Block.empty();
+  visitLoopBranch(HLoopBranch node) {
+    if (node.block == subGraph.end) {
+      // We are generating code for a loop condition.
+      // If we are generating the subgraph as an expression, the
+      // condition will be generated as the expression.
+      // Otherwise, we don't generate the expression, and leave that
+      // to the code that called [visitSubGraph].
+      if (isGeneratingExpression) {
+        use(node.inputs[0]);
+      }
+      return;
+    }
+    if (!node.isDoWhile()) {
+      // For a do while loop, the body has already been visited.
+      visitBasicBlock(node.block.dominatedBlocks[0]);
+    }
+    // The exit block will be visited by the subgraph traversal.
   }
 
-  void endLoop(HBasicBlock block) {
-    js.Statement body = currentContainer;
-    currentContainer = oldContainerStack.removeLast();
-    body = unwrapStatement(body);
-    js.While loop = new js.While(newLiteralBool(true), body);
-
-    HBasicBlock header = block.isLoopHeader() ? block : block.parentLoopHeader;
-    HLoopInformation info = header.loopInformation;
-    attachLocationRange(loop,
-                        info.loopBlockInformation.sourcePosition,
-                        info.loopBlockInformation.endSourcePosition);
-    pushStatement(wrapIntoLabels(loop, info.labels));
-  }
-
-  void handleLoopCondition(HLoopBranch node) {
-    use(node.inputs[0]);
-    js.Expression test = new js.Prefix('!', pop());
-    js.Statement then = new js.Break(null);
-    pushStatement(new js.If.noElse(test, then), node);
-  }
-
-
   void preLabeledBlock(HLabeledBlockInformation labeledBlockInfo) {
   }
 
@@ -2795,6 +2736,36 @@
 
   bool visitAndOrInfo(HAndOrBlockInformation info) => false;
 
+
+  visitLoopBranch(HLoopBranch node) {
+    HBasicBlock header = node.isDoWhile()
+        ? node.block.successors[0]
+        : node.block;
+    if (header.hasBailoutTargets()) {
+      // The graph visitor in [visitLoopInfo] does not handle the
+      // condition. We must instead manually emit it here.
+      handleLoopCondition(node);
+    } else if (node.block == subGraph.end) {

[kasperl, 2013/01/29 18:35:04]

Share this code with the equivalent code above? 

[ngeoffray, 2013/01/30 09:58:53]

Actually I was able to cleanup both visitLoopBranch methods, so this method can
call super.visitLoopBranch directly if the loop body does not have any bailouts.

+      // We are generating code for a loop condition as part
+      // of a SubGraph traversal, the

[kasperl, 2013/01/29 18:35:04]

Reflow comment. SubGraph -> subgraph?

[ngeoffray, 2013/01/30 09:58:53]

Done.

+      // calling code will handle the control flow logic.
+
+      // If we are generating the subgraph as an expression, the
+      // condition will be generated as the expression.
+      // Otherwise, we don't generate the expression, and leave that
+      // to the code that called [visitSubGraph].
+      if (isGeneratingExpression) {
+        use(node.inputs[0]);
+      }
+      return;
+    }
+    if (!node.isDoWhile()) {
+      // For a do while loop, the body has already been visited.
+      visitBasicBlock(node.block.dominatedBlocks[0]);
+    }
+  }
+
+
   bool visitIfInfo(HIfBlockInformation info) {
     if (info.thenGraph.start.hasBailoutTargets()) return false;
     if (info.elseGraph.start.hasBailoutTargets()) return false;
@@ -2802,8 +2773,22 @@
   }
 
   bool visitLoopInfo(HLoopBlockInformation info) {
-    if (info.start.hasBailoutTargets()) return false;
-    if (info.loopHeader.hasBailoutTargets()) return false;
+    // Always emit with block flow traversal.
+    if (info.loopHeader.hasBailoutTargets()) {
+      // If there are any bailout targets in the loop, we cannot use
+      // the pretty [SsaCodeGenerator.visitLoopInfo] printer.
+      if (info.initializer != null) {
+        generateStatements(info.initializer);
+      }
+      beginLoop(info.loopHeader);
+      generateStatements(info.condition);
+      generateStatements(info.body);
+      if (info.updates != null) {
+        generateStatements(info.updates);
+      }
+      endLoop(info.end);
+      return true;
+    }
     return super.visitLoopInfo(info);
   }