Avoid critical edge for loop-exits.

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

Index: sdk/lib/_internal/compiler/implementation/ssa/codegen.dart
diff --git a/sdk/lib/_internal/compiler/implementation/ssa/codegen.dart b/sdk/lib/_internal/compiler/implementation/ssa/codegen.dart
index cdf25f8544806458a08c0095e551b3f6e9c177fa..70f09a6bdad19131db8f522bcebb0387605bf217 100644
--- a/sdk/lib/_internal/compiler/implementation/ssa/codegen.dart
+++ b/sdk/lib/_internal/compiler/implementation/ssa/codegen.dart
@@ -755,6 +755,12 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
     HExpressionInformation condition = info.condition;
     bool isConditionExpression = isJSCondition(condition);
 
+    // Assert that the block we inserted to avoid critical edges satisfies
+    // basic assumptions.
+    assert(!condition.end.successors.last.isEmpty);
+    assert(condition.end.successors.last.first is HGoto);
+    assert(condition.end.successors.last.first.next == null);

[ngeoffray, 2014/06/24 12:39:14]

Should that be in validator.dart instead?

[floitsch, 2014/06/24 14:44:38]

Done.

+
     js.Loop loop;
 
     switch (info.kind) {
@@ -772,7 +778,18 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
             initialization = null;
           }
         }
+
+        // We inserted a basic block to avoid critical edges. This block is
+        // part of the LoopBlockInformation and must therefore be handled here.
+        js.Block oldContainer = currentContainer;
+        js.Block avoidContainer = new js.Block.empty();
+        currentContainer = avoidContainer;
+        assignPhisOfSuccessors(condition.end.successors.last);
+        bool hasPhiUpdates = !avoidContainer.statements.isEmpty;
+        currentContainer = oldContainer;
+
         if (isConditionExpression &&
+            !hasPhiUpdates &&
             info.updates != null && isJSExpression(info.updates)) {
           // If we have an updates graph, and it's expressible as an
           // expression, generate a for-loop.
@@ -828,7 +845,7 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
           // subgraph.
           // TODO(lrn): Remove this extra labeling when handling all loops
           // using subgraphs.
-          js.Block oldContainer = currentContainer;
+          oldContainer = currentContainer;
           js.Statement body = new js.Block.empty();
           currentContainer = body;
           visitBodyIgnoreLabels(info);
@@ -844,7 +861,7 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
           js.Expression jsCondition;
           js.Block oldContainer = currentContainer;
           js.Statement body = new js.Block.empty();
-          if (isConditionExpression) {
+          if (isConditionExpression && !hasPhiUpdates) {
             jsCondition = generateExpression(condition);
             currentContainer = body;
           } else {
@@ -853,8 +870,15 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
             generateStatements(condition);
             use(condition.conditionExpression);
             js.Expression ifTest = new js.Prefix("!", pop());
-            js.Break jsBreak = new js.Break(null);
-            pushStatement(new js.If.noElse(ifTest, jsBreak));
+            js.Statement jsBreak = new js.Break(null);
+            js.Statement exitLoop;
+            if (avoidContainer.statements.isEmpty) {
+              exitLoop = jsBreak;
+            } else {
+              avoidContainer.statements.add(jsBreak);
+              exitLoop = avoidContainer;
+            }
+            pushStatement(new js.If.noElse(ifTest, exitLoop));
           }
           if (info.updates != null) {
             wrapLoopBodyForContinue(info);
@@ -871,7 +895,17 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
         if (info.initializer != null) {
           generateStatements(info.initializer);
         }
+        // We inserted a basic block to avoid critical edges. This block is
+        // part of the LoopBlockInformation and must therefore be handled here.
         js.Block oldContainer = currentContainer;
+        js.Block exitAvoidContainer = new js.Block.empty();
+        currentContainer = exitAvoidContainer;
+        assignPhisOfSuccessors(condition.end.successors.last);
+        bool hasExitPhiUpdates = !exitAvoidContainer.statements.isEmpty;
+        currentContainer = oldContainer;
+
+
+        oldContainer = currentContainer;
         js.Block body = new js.Block.empty();
         // If there are phi copies in the block that jumps to the
         // loop entry, we must emit the condition like this:
@@ -907,10 +941,18 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
           // at the end of the loop anyway.
           loop = new js.While(newLiteralBool(true), unwrapStatement(body));
         } else {
-          if (hasPhiUpdates) {
+          if (hasPhiUpdates || hasExitPhiUpdates) {
             updateBody.statements.add(new js.Continue(null));
+            js.Statement jsBreak = new js.Break(null);
+            js.Statement exitLoop;
+            if (exitAvoidContainer.statements.isEmpty) {
+              exitLoop = jsBreak;
+            } else {
+              exitAvoidContainer.statements.add(jsBreak);
+              exitLoop = exitAvoidContainer;
+            }
             body.statements.add(
-                new js.If(jsCondition, updateBody, new js.Break(null)));
+                new js.If(jsCondition, updateBody, exitLoop));
             jsCondition = newLiteralBool(true);
           }
           loop = new js.Do(unwrapStatement(body), jsCondition);