Fix another bailout problem where not all blocks were visiting when propagating bailout information…

sdk/lib/_internal/compiler/implementation/ssa/bailout.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 BailoutInfo {
   int instructionId;
   int bailoutId;
   BailoutInfo(this.instructionId, this.bailoutId);
@@ skipping 376 matching lines @@
 }
 
 /**
  * Propagates bailout information to blocks that need it. This visitor
  * is run before codegen, to know which blocks have to deal with
  * bailouts.
  */
 class SsaBailoutPropagator extends HBaseVisitor {
   final Compiler compiler;
   final List<HBasicBlock> blocks;
-  final List<HLabeledBlockInformation> labeledBlockInformations;
-  final Set<HInstruction> generateAtUseSite;
   SubGraph subGraph;
+  HBlockInformation currentBlockInformation;
+
   /**
    * Max number of arguments to the bailout (not counting the state).
    */
   int bailoutArity;
   /**
    * A map from variables to their names.  These are the names in the
    * unoptimized (bailout) version of the function.  Their names could be
    * different in the optimized version.
    */
   VariableNames variableNames;
@@ skipping 17 matching lines @@
    * The first type guard in the graph.
    */
   HBailoutTarget firstBailoutTarget;
 
   /**
    * If set, it is the first block in the graph where we generate
    * code. Blocks before this one are dead code in the bailout
    * version.
    */
 
-  SsaBailoutPropagator(this.compiler,
-                       this.generateAtUseSite,
-                       this.variableNames)
+  SsaBailoutPropagator(this.compiler, this.variableNames)
       : blocks = <HBasicBlock>[],
-        labeledBlockInformations = <HLabeledBlockInformation>[],
         bailoutArity = 0,
         parameterNames = new Map<String, int>();
 
   void visitGraph(HGraph graph) {
     subGraph = new SubGraph(graph.entry, graph.exit);
     visitBasicBlock(graph.entry);
     if (!blocks.isEmpty) {
       compiler.internalError('Bailout propagation',
           node: compiler.currentElement.parseNode(compiler));
     }
   }
 
+  bool handleBlockFlow(HBlockFlow blockFlow) {

[kasperl, 2013/01/31 01:50:06]

This method deserves a comment. 

[ngeoffray, 2013/01/31 08:17:24]

Done.

+    HBlockInformation body = blockFlow.body;
+
+    // We reach here again when starting to visit a subgraph. Just
+    // return to visiting the block.
+    if (currentBlockInformation == body) return false;
+
+    HBlockInformation oldInformation = currentBlockInformation;
+    if (body is HLabeledBlockInformation) {
+      currentBlockInformation = body;
+      HLabeledBlockInformation info = body;
+      visitStatements(info.body, newFlow: true);
+    } else if (body is HLoopBlockInformation) {
+      currentBlockInformation = body;
+      HLoopBlockInformation info = body;
+      if (info.initializer != null) {
+        visitExpression(info.initializer);
+      }
+      blocks.addLast(info.loopHeader);
+      if (!info.isDoWhile()) {
+        visitExpression(info.condition);
+      }
+      visitStatements(info.body, newFlow: false);
+      if (info.isDoWhile()) {
+        visitExpression(info.condition);
+      }
+      if (info.updates != null) {
+        visitExpression(info.updates);
+      }
+      blocks.removeLast();
+    } else {
+      assert(body is! HTryBlockInformation);
+      assert(body is! HSwitchBlockInformation);
+      // [HIfBlockInformation] is handled by visitIf.
+      return false;
+    }
+
+    currentBlockInformation = oldInformation;
+    if (blockFlow.continuation != null) {
+      visitBasicBlock(blockFlow.continuation);
+    }
+    return true;
+  }
+
   void visitBasicBlock(HBasicBlock block) {
     // Abort traversal if we are leaving the currently active sub-graph.
     if (!subGraph.contains(block)) return;
 
-    if (block.isLoopHeader()) {
-      blocks.addLast(block);
-    } else if (block.isLabeledBlock()
-               && (blocks.isEmpty || !identical(blocks.last, block))) {
-      HLabeledBlockInformation info = block.blockFlow.body;
-      visitStatements(info.body);
-      return;
-    }
+    HBlockFlow blockFlow = block.blockFlow;
+    if (blockFlow != null && handleBlockFlow(blockFlow)) return;
 
     HInstruction instruction = block.first;
     while (instruction != null) {
       instruction.accept(this);
       instruction = instruction.next;
     }
   }
 
-  void visitStatements(HStatementInformation info) {
-    assert(info is HSubGraphBlockInformation);
-    HSubGraphBlockInformation graph = info;
-    visitSubGraph(graph.subGraph);
+  void visitExpression(HSubExpressionBlockInformation info) {
+    visitSubGraph(info.subExpression);
+  }
+
+  void visitStatements(HSubGraphBlockInformation info, {bool newFlow}) {

[kasperl, 2013/01/31 01:50:06]

Add a comment that explains how this works and what the semantics of 'newFlow'
are.

[ngeoffray, 2013/01/31 08:17:24]

Done.

+    SubGraph graph = info.subGraph;
+    if (newFlow) blocks.addLast(graph.start);
+    visitSubGraph(graph);
+    if (newFlow) blocks.removeLast();
   }
 
   void visitSubGraph(SubGraph graph) {
     SubGraph oldSubGraph = subGraph;
     subGraph = graph;
-    HBasicBlock start = graph.start;
-    blocks.addLast(start);
-    visitBasicBlock(start);
-    blocks.removeLast();
+    visitBasicBlock(graph.start);
     subGraph = oldSubGraph;
-
-    if (start.isLabeledBlock()) {
-      HBasicBlock continuation = start.blockFlow.continuation;
-      if (continuation != null) {
-        visitBasicBlock(continuation);
-      }
-    }
   }
 
   void visitIf(HIf instruction) {
     int preVisitedBlocks = 0;
     HIfBlockInformation info = instruction.blockInformation.body;
-    visitStatements(info.thenGraph);
+    visitStatements(info.thenGraph, newFlow: true);
     preVisitedBlocks++;
-    visitStatements(info.elseGraph);
+    visitStatements(info.elseGraph, newFlow: true);
     preVisitedBlocks++;
 
     HBasicBlock joinBlock = instruction.joinBlock;
     if (joinBlock != null
         && !identical(joinBlock.dominator, instruction.block)) {
       // The join block is dominated by a block in one of the branches.
       // The subgraph traversal never reached it, so we visit it here
       // instead.
       visitBasicBlock(joinBlock);
     }
@@ skipping 12 matching lines @@
 
   void visitGoto(HGoto goto) {
     HBasicBlock block = goto.block;
     HBasicBlock successor = block.successors[0];
     if (identical(successor.dominator, block)) {
       visitBasicBlock(block.successors[0]);
     }
   }
 
   void visitLoopBranch(HLoopBranch branch) {
-    HBasicBlock branchBlock = branch.block;
-    List<HBasicBlock> dominated = branchBlock.dominatedBlocks;
     // For a do-while loop, the body has already been visited.
     if (!branch.isDoWhile()) {
-      visitBasicBlock(dominated[0]);
-    }
-    blocks.removeLast();
-
-    // 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]);
-      }
+      visitBasicBlock(branch.block.dominatedBlocks[0]);
     }
   }
 
   visitBailoutTarget(HBailoutTarget target) {
     int inputLength = target.inputs.length;
     for (HInstruction input in target.inputs) {
       String inputName = variableNames.getName(input);
       int position = parameterNames[inputName];
       if (position == null) {
         position = parameterNames[inputName] = bailoutArity++;
       }
     }
 
     if (blocks.isEmpty) {
       if (firstBailoutTarget == null) {
         firstBailoutTarget = target;
       } else {
         hasComplexBailoutTargets = true;
       }
     } else {
       hasComplexBailoutTargets = true;
       blocks.forEach((HBasicBlock block) {
         block.bailoutTargets.add(target);
       });
     }
   }
 }