Generate nested else if

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

Index: dart/lib/compiler/implementation/ssa/codegen.dart
===================================================================
--- dart/lib/compiler/implementation/ssa/codegen.dart	(revision 8661)
+++ dart/lib/compiler/implementation/ssa/codegen.dart	(working copy)
@@ -123,7 +123,8 @@
   static final int ONE_STATEMENT = 0;
   static final int ONE_EXPRESSION = 1;
   static final int EMPTY = 2;
-  static final int MULTIPLE_STATEMENTS = 3;
+  static final int IF_STATEMENT = 3;
+  static final int MULTIPLE_STATEMENTS = 4;
 
   /**
    * Returned by [expressionType] to tell how code can be generated for
@@ -1315,12 +1316,10 @@
    * contains one statement, one expression, or multiple statements.
    */
   int analyzeGraphForCodegen(HStatementInformation graph) {
-    HBasicBlock start = graph.start;
-    HBasicBlock end = graph.end;
-    // Only deal with single blocks for now. TODO(ngeoffray): analyze
-    // all blocks.
-    if (start !== end) return MULTIPLE_STATEMENTS;
+    return analyzeBlocksForCodegen(graph.start, graph.end);
+  }
 
+  int analyzeBlocksForCodegen(HBasicBlock start, HBasicBlock end) {
     int kind = EMPTY;
     bool updateKind(int newKind) {
       if (kind != EMPTY) return false;
@@ -1339,10 +1338,25 @@
     }
 
     HInstruction last = start.last;
-    if (last is !HGoto) {
-      if (!updateKind(last.isStatement() ? ONE_STATEMENT : ONE_EXPRESSION)) {
+    if (last is HGoto) {
+      if (start !== end) {
+        int nextKind = analyzeBlocksForCodegen(start.successors[0], end);
+        if (!updateKind(nextKind)) return MULTIPLE_STATEMENTS;
+      }
+    } else if (last is HIf) {
+      HIf ifInstruction = last;
+      if (ifInstruction.joinBlock !== null
+          && analyzeBlocksForCodegen(ifInstruction.joinBlock, end) != EMPTY) {
         return MULTIPLE_STATEMENTS;
       }
+      int ifKind = controlFlowOperators.contains(ifInstruction)
+          ? ONE_EXPRESSION
+          : IF_STATEMENT;

[Lasse Reichstein Nielsen, 2012/06/15 08:31:01]

Maybe (later) consider whether the if-"statement" could be a conditional
expression.

+      if (!updateKind(ifKind)) return MULTIPLE_STATEMENTS;
+    } else if (start !== end) {
+      return MULTIPLE_STATEMENTS;
+    } else if (!updateKind(last.isStatement() ? ONE_STATEMENT : ONE_EXPRESSION)) {
+      return MULTIPLE_STATEMENTS;
     }
 
     CopyHandler handler = variableNames.getCopyHandler(start);
@@ -1376,6 +1390,7 @@
     // Usually, the variable name is longer than 'if' and it takes up
     // more space to duplicate the name.
     if (!atUseSite
+        && !generatingInlineStatement
         && variableNames.getName(phi) == variableNames.getName(phi.inputs[1])) {
       return false;
     }
@@ -1391,16 +1406,14 @@
     int elseKind = analyzeGraphForCodegen(elseGraph);
 
     void visitWithoutIndent(HStatementInformation toVisit) {
-      int oldIndent = indent;
-      indent = 0;
-      generateStatements(toVisit);
-      indent = oldIndent;
+      generatingInlineStatement = true;
+      visitSubGraph(new SubGraph(toVisit.start, toVisit.end));
     }
 
     void visitWithIndent(HStatementInformation toVisit) {
       buffer.add('{\n');
       indent++;
-      generateStatements(toVisit);
+      visitSubGraph(new SubGraph(toVisit.start, toVisit.end));
       indent--;
       addIndented('}');
     }
@@ -1444,6 +1457,7 @@
             break;
 
           case ONE_STATEMENT:
+          case IF_STATEMENT:
             addIndented('if (');
             generateNot(node.inputs[0]);
             buffer.add(') ');
@@ -1476,6 +1490,7 @@
 
           case ONE_EXPRESSION:
           case ONE_STATEMENT:
+          case IF_STATEMENT:
             // TODO(ngeoffray): Generate a conditional.
             emitIf();
             visitWithoutIndent(thenGraph);
@@ -1502,6 +1517,7 @@
         break;
 
       case MULTIPLE_STATEMENTS:
+      case IF_STATEMENT:
         emitIf();
         visitWithIndent(thenGraph);
 
@@ -1512,6 +1528,7 @@
 
           case ONE_EXPRESSION:
           case ONE_STATEMENT:
+          case IF_STATEMENT:
             if (thenGraphHasSuccessor) {
               buffer.add(' else ');
               visitWithoutIndent(elseGraph);
@@ -1554,8 +1571,18 @@
       HConstant constant = condition;
       if (constant.constant.isTrue()) {
         generateStatements(info.thenGraph);
+        int thenKind = analyzeGraphForCodegen(info.thenGraph);
+        if (thenKind == EMPTY && generatingInlineStatement) {
+          generatingInlineStatement = false;

[Lasse Reichstein Nielsen, 2012/06/15 08:31:01]

Explain why you set generatingInlineStatement to false
(this might be obvious if the variable was documented).

[ngeoffray, 2012/06/18 15:43:25]

Done.

+          buffer.add(';\n');
+        }
       } else {
         generateStatements(info.elseGraph);
+        int elseKind = analyzeGraphForCodegen(info.elseGraph);
+        if (elseKind == EMPTY && generatingInlineStatement) {
+          generatingInlineStatement = false;
+          buffer.add(';\n');
+        }
       }
     } else {
       generateIf(node, info);
@@ -2002,9 +2029,14 @@
     endExpression(JSPrecedence.EXPRESSION_PRECEDENCE);
   }
 
+  bool generatingInlineStatement = false;

[Lasse Reichstein Nielsen, 2012/06/15 08:31:01]

Move this field to the top of the class, and document what it means and when
it's used.

[ngeoffray, 2012/06/18 15:43:25]

Done.

   void addIndentation() {
-    for (int i = 0; i < indent; i++) {
-      buffer.add('  ');
+    if (generatingInlineStatement) {
+      generatingInlineStatement = false;
+    } else {
+      for (int i = 0; i < indent; i++) {
+        buffer.add('  ');
+      }
     }
   }