Refactoring if block-information.

lib/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.
 
 class SsaCodeGeneratorTask extends CompilerTask {
   SsaCodeGeneratorTask(Compiler compiler) : super(compiler);
   String get name() => 'SSA code generator';
 
 
   String generateMethod(WorkItem work, HGraph graph) {
@@ skipping 42 matching lines @@
       parameterNames[element] = function.isNative()
           ? element.name.slowToString()
           : JsNames.getValid('${element.name.slowToString()}');
     });
     return parameterNames;
   }
 }
 
 typedef void ElementAction(Element element);
 
-class SsaCodeGenerator implements HVisitor {
+class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
   /**
    * Current state for generating simple (non-local-control) code.
    * It is generated as either statements (indented and ';'-terminated),
    * expressions (comma separated) or declarations (also comma separated,
    * but expected to be preceeded by a 'var' so it declares its variables);
    */
   static final int STATE_STATEMENT = 0;
   static final int STATE_FIRST_EXPRESSION = 1;
   static final int STATE_FIRST_DECLARATION = 2;
   static final int STATE_EXPRESSION = 3;
@@ skipping 312 matching lines @@
     buffer.add(";\n");
   }
 
   void implicitBreakWithLabel(TargetElement target) {
     addIndentation();
     buffer.add("break ");
     writeImplicitLabel(target);
     buffer.add(";\n");
   }
 
-  void handleLabeledBlock(HLabeledBlockInformation labeledBlockInfo) {
+  bool visitIfInfo(HIfBlockInformation info) {
+    return false;
+  }
+
+  bool visitAndOrInfo(HAndOrBlockInformation info) {
+    return false;
+  }
+
+  bool visitLoopInfo(HLoopInformation info) {
+    bool success = false;
+    SubExpression condition = info.condition;
+    void visitBodyIgnoreLabels() {
+      if (info.body.start.isLabeledBlock()) {
+        HBlockInformation oldInfo = currentBlockInformation;
+        currentBlockInformation = info.body.start.blockInformation;
+        visitSubGraph(info.body);
+        currentBlockInformation = oldInfo;
+      } else {
+        visitSubGraph(info.body);
+      }
+    }
+    if (isCondition(condition)) {

[floitsch, 2012/04/16 13:26:07]

I would prefer bailing out here.

if (!isCondition(condition)) return false;
if (info.type == HLoopInformation.DO_WHILE_LOOP) return false;

[Lasse Reichstein Nielsen, 2012/04/17 12:58:22]

Done.

+      switch (info.type) {

[floitsch, 2012/04/16 13:26:07]

I would prefer 'kind' instead of 'type'.

[Lasse Reichstein Nielsen, 2012/04/17 12:58:22]

Done.

+        case HLoopInformation.WHILE_LOOP:
+        case HLoopInformation.FOR_IN_LOOP: {

[floitsch, 2012/04/16 13:26:07]

Afaics there is almost no difference between while, for-in, and for. Why is the
for-loop treated specially?

[Lasse Reichstein Nielsen, 2012/04/17 12:58:22]

It can have an update block that isn't just phis.
And an initialization part, that I want to put in the for-structure eventually.
But true, the difference isn't big.

+          addIndentation();
+          for (LabelElement label in info.labels) {
+            writeLabel(label);
+            buffer.add(":");
+          }
+          bool inlineUpdates =
+              info.updates !== null && isExpression(info.updates);
+          if (inlineUpdates) {
+            buffer.add("for (; ");
+            visitConditionGraph(condition);
+            buffer.add("; ");
+            visitExpressionGraph(info.updates);
+            buffer.add(") {\n");
+            indent++;
+            // The body might be labeled. Ignore this when recursing on the
+            // subgraph.
+            // TODO(lrn): Remove this extra labeling when handling all loops
+            // using subgraphs.
+            visitBodyIgnoreLabels();
+
+            indent--;
+          } else {
+            buffer.add("while (");
+            visitConditionGraph(condition);
+            buffer.add(") {\n");
+            indent++;
+            wrapLoopBodyForContinue(info);
+            if (info.updates !== null) visitSubGraph(info.updates);
+            indent--;
+          }
+          addIndentation();
+          buffer.add("}\n");
+          success = true;
+          break;
+        }
+        case HLoopInformation.FOR_LOOP: {
+          // TODO(lrn): Find a way to put initialization into the for.
+          // It's currently handled before we reach the [HLoopInformation].
+          addIndentation();
+          for (LabelElement label in info.labels) {
+            if (label.isTarget) {
+              writeLabel(label);
+              buffer.add(":");
+            }
+          }
+          buffer.add("for(;");
+          visitConditionGraph(info.condition);
+          buffer.add(";");
+          if (isExpression(info.updates)) {
+            visitExpressionGraph(info.updates);
+            buffer.add(") {\n");
+            indent++;
+
+            visitBodyIgnoreLabels();
+
+            indent--;
+            addIndentation();
+            buffer.add("}\n");
+          } else {
+            addIndentation();
+            buffer.add(") {\n");
+            indent++;
+            wrapLoopBodyForContinue(info);
+            visitSubGraph(info.updates);
+            indent--;
+            buffer.add("}\n");
+          }
+          success = true;
+          break;
+        }
+        case HLoopInformation.DO_WHILE_LOOP:
+          // Currently unhandled.

[floitsch, 2012/04/16 13:26:07]

We don't have fall-throughs. Why didn't this through an error?

[Lasse Reichstein Nielsen, 2012/04/17 12:58:22]

It's an empty case, so it's joined with the following default.
Comments are not significant.

+        default:

[floitsch, 2012/04/16 13:26:07]

Is default an internal error?

[Lasse Reichstein Nielsen, 2012/04/17 12:58:22]

Probably, yes. I'll make it an internal error, and just return false from
do-while.

+      }
+    }
+    if (success) {

[floitsch, 2012/04/16 13:26:07]

Unless wrong you can merge while, for-in and for. But even if you can't, please
move this into the respective cases.

[Lasse Reichstein Nielsen, 2012/04/17 12:58:22]

I'll keep it here, but without the if (I'll return false immediately when I bail
out).

+      visitBasicBlock(info.joinBlock);
+      return true;
+    }
+    return false;
+  }
+
+  bool visitLabeledBlockInfo(HLabeledBlockInformation labeledBlockInfo) {
     addIndentation();
     Link<Element> continueOverrides = const EmptyLink<Element>();
     // If [labeledBlockInfo.isContinue], the block is an artificial
     // block around the body of a loop with an update block, so that
     // continues of the loop can be written as breaks of the body
     // block.
     if (labeledBlockInfo.isContinue) {
       for (LabelElement label in labeledBlockInfo.labels) {
         if (label.isContinueTarget) {
           writeContinueLabel(label);
@@ skipping 40 matching lines @@
       visitBasicBlock(labeledBlockInfo.joinBlock);
     }
     if (labeledBlockInfo.isContinue) {
       while (!continueOverrides.isEmpty()) {
         continueAction.remove(continueOverrides.head);
         continueOverrides = continueOverrides.tail;
       }
     } else {
       breakAction.remove(labeledBlockInfo.target);
     }
+    return true;
   }
 
   void emitLogicalOperation(HPhi node, String operation) {
     JSBinaryOperatorPrecedence operatorPrecedence =
         JSPrecedence.binary[operation];
     beginExpression(operatorPrecedence.precedence);
     use(node.inputs[0], operatorPrecedence.left);
     buffer.add(" $operation ");
     use(node.inputs[1], operatorPrecedence.right);
     endExpression(operatorPrecedence.precedence);
@@ skipping 25 matching lines @@
         if (label.isContinueTarget) {
           continueAction.remove(label);
         }
       }
     } else {
       // Loop body contains no continues, so we don't need a break target.
       visitSubGraph(info.body);
     }
   }
 
-  bool handleLoop(HBasicBlock node) {
-    bool success = false;
-    assert(node.isLoopHeader());
-    HLoopInformation info = node.loopInformation;
-    SubExpression condition = info.condition;
-    if (isCondition(condition)) {
-      switch (info.type) {
-        case HLoopInformation.WHILE_LOOP:
-        case HLoopInformation.FOR_IN_LOOP: {
-          addIndentation();
-          for (LabelElement label in info.labels) {
-            writeLabel(label);
-            buffer.add(":");
-          }
-          bool inlineUpdates =
-              info.updates !== null && isExpression(info.updates);
-          if (inlineUpdates) {
-            buffer.add("for (; ");
-            visitConditionGraph(condition);
-            buffer.add("; ");
-            visitExpressionGraph(info.updates);
-            buffer.add(") {\n");
-            indent++;
-            // The body might be labeled. Ignore this when recursing on the
-            // subgraph.
-            // TODO(lrn): Remove this extra labeling when handling all loops
-            // using subgraphs.
-            HBlockInformation oldInfo = currentBlockInformation;
-            currentBlockInformation = info.body.start.labeledBlockInformation;
-            visitSubGraph(info.body);
-            currentBlockInformation = oldInfo;
-
-            indent--;
-          } else {
-            buffer.add("while (");
-            visitConditionGraph(condition);
-            buffer.add(") {\n");
-            indent++;
-            wrapLoopBodyForContinue(info);
-            if (info.updates !== null) visitSubGraph(info.updates);
-            indent--;
-          }
-          addIndentation();
-          buffer.add("}\n");
-          success = true;
-          break;
-        }
-        case HLoopInformation.FOR_LOOP: {
-          // TODO(lrn): Find a way to put initialization into the for.
-          // It's currently handled before we reach the [HLoopInformation].
-          addIndentation();
-          for (LabelElement label in info.labels) {
-            if (label.isTarget) {
-              writeLabel(label);
-              buffer.add(":");
-            }
-          }
-          buffer.add("for(;");
-          visitConditionGraph(info.condition);
-          buffer.add(";");
-          if (isExpression(info.updates)) {
-            visitExpressionGraph(info.updates);
-            buffer.add(") {\n");
-            indent++;
-
-            HBlockInformation oldInfo = currentBlockInformation;
-            currentBlockInformation = info.body.start.labeledBlockInformation;
-            visitSubGraph(info.body);
-            currentBlockInformation = oldInfo;
-
-            indent--;
-            addIndentation();
-            buffer.add("}\n");
-          } else {
-            addIndentation();
-            buffer.add(") {\n");
-            indent++;
-            wrapLoopBodyForContinue(info);
-            visitSubGraph(info.updates);
-            indent--;
-            buffer.add("}\n");
-          }
-          success = true;
-          break;
-        }
-        case HLoopInformation.DO_WHILE_LOOP:
-          // Currently unhandled.
-        default:
-      }
-    }
-    return success;
-  }
-
   void visitBasicBlock(HBasicBlock node) {
     // Abort traversal if we are leaving the currently active sub-graph.
     if (!subGraph.contains(node)) return;
 
     // If this node has special behavior attached, handle it.
     // If we reach here again while handling the attached information,
     // e.g., because we call visitSubGraph on a subgraph starting here,
     // don't handle it again.
-    if (node.hasLabeledBlockInformation() &&
-        node.labeledBlockInformation !== currentBlockInformation) {
+    if (node.blockInformation !== null &&
+        node.blockInformation !== currentBlockInformation) {
       HBlockInformation oldBlockInformation = currentBlockInformation;
-      currentBlockInformation = node.labeledBlockInformation;
-      handleLabeledBlock(currentBlockInformation);
+      currentBlockInformation = node.blockInformation;
+      bool success = currentBlockInformation.accept(this);
       currentBlockInformation = oldBlockInformation;
-      return;
+      if (success) return;
+
+      // Even if our special handling didn't succeed, loop blocks

[floitsch, 2012/04/16 13:26:07]

If our special handling didn't succeed, we have to emit a generic version. There
is hence still special handling for loop-blocks.

[Lasse Reichstein Nielsen, 2012/04/17 12:58:22]

Done.

+      // have special treatement in the primitive traversal too.
+      if (node.isLoopHeader()) {
+        beginLoop(node);
+      }
     }
-
-    if (node.isLoopHeader() &&
-        node.loopInformation !== currentBlockInformation) {
-      HBlockInformation oldBlockInformation = currentBlockInformation;
-      currentBlockInformation = node.loopInformation;
-      bool prettyLoop = handleLoop(node);
-      currentBlockInformation = oldBlockInformation;
-      if (prettyLoop) {
-        visitBasicBlock(node.loopInformation.joinBlock);
-        return;
-      }
-      beginLoop(node);
-    }
-
     iterateBasicBlock(node);
   }
 
   void iterateBasicBlock(HBasicBlock node) {
     currentBlock = node;
     HInstruction instruction = node.first;
     while (instruction != null) {
       if (instruction === node.last) {
         for (HBasicBlock successor in node.successors) {
           int index = successor.predecessors.indexOf(node);
@@ skipping 949 matching lines @@
       buffer.add(') ');
       bailout(node, 'Not a string');
     } else if (node.isMutableArray()) {
       buffer.add('if (');
       checkObject(input, '!==');
       buffer.add('||');
       checkArray(input, '!==');
       buffer.add('||');
       checkImmutableArray(input);
       buffer.add(') ');
-      bailout(node, 'Not a mutable array');      
+      bailout(node, 'Not a mutable array');
     } else if (node.isArray()) {
       buffer.add('if (');
       checkObject(input, '!==');
       buffer.add('||');
       checkArray(input, '!==');
       buffer.add(') ');
       bailout(node, 'Not an array');
     } else if (node.isStringOrArray()) {
       buffer.add('if (');
       checkString(input, '!==');
       buffer.add(' && (');
       checkObject(input, '!==');
       buffer.add('||');
       checkArray(input, '!==');
       buffer.add(')) ');
       bailout(node, 'Not a string or array');
     } else {
       unreachable();
     }
     buffer.add(';\n');
   }
 
   void beginLoop(HBasicBlock block) {
     addIndentation();
-    for (LabelElement label in block.loopInformation.labels) {
+    HLoopInformation info = block.blockInformation;
+    for (LabelElement label in info.labels) {
       writeLabel(label);
       buffer.add(":");
     }
     buffer.add('while (true) {\n');
     indent++;
   }
 
   void endLoop(HBasicBlock block) {
     indent--;
     addIndentation();
@@ skipping 98 matching lines @@
       HBoundsCheck instruction = argument;
       return unwrap(instruction.index);
     } else if (argument is HTypeGuard) {
       HTypeGuard instruction = argument;
       return unwrap(instruction.guarded);
     } else {
       return argument;
     }
   }
 
-  bool handleLoop(HBasicBlock node) => false;
+  bool visitLoopInfo(HLoopInformation info) => false;
 
   void visitTypeGuard(HTypeGuard node) {
     indent--;
     addIndentation();
     buffer.add('case ${node.state}:\n');
     indent++;
     addIndentation();
     buffer.add('state = 0;\n');
 
     setup.add('    case ${node.state}:\n');
@@ skipping 31 matching lines @@
     indent++;
   }
 
   void endBailoutSwitch() {
     indent--; // Close 'case'.
     indent--;
     addIndentation();
     buffer.add('}\n');  // Close 'switch'.
   }
 
-
   void beginLoop(HBasicBlock block) {
     // TODO(ngeoffray): Don't put labels on loops that don't bailout.
     String newLabel = pushLabel();
     if (block.hasGuards()) {
       startBailoutCase(block.guards, const <HTypeGuard>[]);
     }
 
     addIndentation();
-    for (SourceString label in block.loopInformation.labels) {
+    HLoopInformation info = block.blockInformation;
+    for (SourceString label in info.labels) {
       writeLabel(label);
       buffer.add(":");
     }
     buffer.add('$newLabel: while (true) {\n');
     indent++;
 
     if (block.hasGuards()) {
       startBailoutSwitch();
     }
   }
@@ skipping 72 matching lines @@
       startBailoutSwitch();
     }
   }
 
   void endElse(HIf node) {
     if (node.elseBlock.hasGuards()) {
       endBailoutSwitch();
     }
   }
 }