dart2js cps: Emit 'return' after throwing call to assist VM analysis.

pkg/compiler/lib/src/js_backend/codegen/codegen.dart

Index: pkg/compiler/lib/src/js_backend/codegen/codegen.dart
diff --git a/pkg/compiler/lib/src/js_backend/codegen/codegen.dart b/pkg/compiler/lib/src/js_backend/codegen/codegen.dart
index bcf09683b88188eb9cd796c250bc132d3264e51d..a244edab913613caab31e8e6968ae3bc084ac137 100644
--- a/pkg/compiler/lib/src/js_backend/codegen/codegen.dart
+++ b/pkg/compiler/lib/src/js_backend/codegen/codegen.dart
@@ -533,9 +533,14 @@ class CodeGenerator extends tree_ir.StatementVisitor
 
   @override
   void visitExpressionStatement(tree_ir.ExpressionStatement node) {
-    accumulator.add(new js.ExpressionStatement(
-        visitExpression(node.expression)));
-    visitStatement(node.next);
+    js.Expression exp = visitExpression(node.expression);
+    if (node.next is tree_ir.Unreachable) {

[sra1, 2015/11/30 18:59:40]

Can we tell if the unreachable code is not conditional?
Add a TODO if you don't want to tackle the following right now.

The exit is only valuable to the JS VM if the exit is conditional and the
condition constrains variables that are used after the test.

The following is a trivial example of useless exit insertion:

    ConstantMap: {
       ...
      $indexSet: function(receiver, key, val) {
        H.ConstantMap__throwUnmodifiable();
      },
--->
      $indexSet: function(receiver, key, val) {
        return H.ConstantMap__throwUnmodifiable();
      },


Slightly less trivial: conditional but no following code

      _eatEnd$0: function() {
        this._maybeEatWhitespaceOrComments$0();
        if (!this.get$endOfInput())
          this._layout$_error$1("expected end of input");
      },
--->
      _eatEnd$0: function() {
        this._maybeEatWhitespaceOrComments$0();
        if (!this.get$endOfInput())
          return this._layout$_error$1("expected end of input");
      },

[asgerf, 2015/12/01 12:40:20]

The easy fix was to look at the fallthrough stack, i.e. something like:

  if (isNullReturn(fallthough.target)) { 
    fallthrough.use();
  } else {
    return new js.Return(...);
  }

But this made a lot of code look awfully bad, for example:

  if (check)
    return ioore;
  following_code;

==>

  if (check)
    ioore;
  else {
    following_code;
  }

The short-if optimizations in visitIf needs to have higher priority than
omitting an unreachable 'return'.

There is now a separate stack to keep track of whether an unreachable node
should be emitted as 'return', which handles the two examples and without
disabling the short-if optimizations.

+      // Emit as 'return exp' to assist local analysis in the VM.
+      accumulator.add(new js.Return(exp));
+    } else {
+      accumulator.add(new js.ExpressionStatement(exp));
+      visitStatement(node.next);
+    }
   }
 
   @override
@@ -682,8 +687,8 @@ class CodeGenerator extends tree_ir.StatementVisitor
 
   @override
   void visitUnreachable(tree_ir.Unreachable node) {
-    // Output nothing.
-    // TODO(asgerf): Emit a throw/return to assist local analysis in the VM?
+    // Emit a return to assist local analysis in the VM.
+    accumulator.add(new js.Return());
   }
 
   @override