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

 // Copyright (c) 2012, 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 SsaCodeGeneratorTask extends CompilerTask {
 
   final JavaScriptBackend backend;
 
@@ skipping 319 matching lines @@
     jsNode.endSourcePosition = endSourcePosition;
     return jsNode;
   }
 
   visitTypeGuard(HTypeGuard node);
   visitBailoutTarget(HBailoutTarget node);
 
   beginGraph(HGraph graph);
   endGraph(HGraph graph);
 
-  beginLoop(HBasicBlock block);
-  endLoop(HBasicBlock block);
-  handleLoopCondition(HLoopBranch node);
-
   preLabeledBlock(HLabeledBlockInformation labeledBlockInfo);
   startLabeledBlock(HLabeledBlockInformation labeledBlockInfo);
   endLabeledBlock(HLabeledBlockInformation labeledBlockInfo);
 
   void preGenerateMethod(HGraph graph) {
     new SsaInstructionMerger(types, generateAtUseSite).visitGraph(graph);
     new SsaConditionMerger(
         types, generateAtUseSite, controlFlowOperators).visitGraph(graph);
     SsaLiveIntervalBuilder intervalBuilder =
         new SsaLiveIntervalBuilder(compiler, generateAtUseSite);
@@ skipping 598 matching lines @@
         //   } else {
         //     break;
         //   }
         // } while (true);
         HBasicBlock avoidEdge = info.end.successors[0];
         js.Block updateBody = new js.Block.empty();
         currentContainer = updateBody;
         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);
         }
         if (isConditionExpression) {
           push(generateExpression(condition));
         } else {
           generateStatements(condition);
           use(condition.conditionExpression);
         }
         js.Expression jsCondition = pop();
@@ skipping 141 matching lines @@
     return success;
   }
 
   void visitBasicBlock(HBasicBlock node) {
     // Abort traversal if we are leaving the currently active sub-graph.
     if (!subGraph.contains(node)) return;
 
     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);
   }
 
   void emitAssignment(String destination, String source) {
     assignVariable(destination, new js.VariableUse(source));
   }
 
   /**
    * Sequentialize a list of conceptually parallel copies. Parallel
    * copies may contain cycles, that this method breaks.
@@ skipping 679 matching lines @@
     assert(isGenerateAtUseSite(node));
     generateConstant(node.constant);
     DartType type = node.constant.computeType(compiler);
     if (node.constant is ConstructedConstant) {
       ConstantHandler handler = compiler.constantHandler;
       handler.registerCompileTimeConstant(node.constant);
     }
     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]);
     attachLocationToLast(node);
   }
 
   void generateNot(HInstruction input) {
     bool canGenerateOptimizedComparison(HInstruction instruction) {
       if (instruction is !HRelational) return false;
       HRelational relational = instruction;
@@ skipping 765 matching lines @@
       pushStatement(new js.If.noElse(test, then), node);
     } else {
       compiler.internalError('Unexpected type guard', instruction: input);
     }
   }
 
   void visitBailoutTarget(HBailoutTarget target) {
     // 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) {
   }
 
   void startLabeledBlock(HLabeledBlockInformation labeledBlockInfo) {
   }
 
   void endLabeledBlock(HLabeledBlockInformation labeledBlockInfo) {
   }
 }
 
@@ skipping 93 matching lines @@
     }
   }
 
   visitParameterValue(HParameterValue node) {
     // Nothing to do, parameters are dealt with specially in a bailout
     // method.
   }
 
   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;
     return super.visitIfInfo(info);
   }
 
   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);
   }
 
   bool visitTryInfo(HTryBlockInformation info) => false;
   bool visitSequenceInfo(HStatementSequenceInformation info) => false;
 
   void visitTypeGuard(HTypeGuard node) {
     // Do nothing. Type guards are only used in the optimized version.
   }
 
@@ skipping 211 matching lines @@
   if (leftType.canBeNull() && rightType.canBeNull()) {
     if (left.isConstantNull() || right.isConstantNull() ||
         (leftType.isPrimitive() && leftType == rightType)) {
       return '==';
     }
     return null;
   } else {
     return '===';
   }
 }