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

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

BUG=
R=sra@google.com

Committed: https://github.com/dart-lang/sdk/commit/91e2862a98b5aeb8bdeca729e1de60af7b853695

[asgerf, 2015-11-27 10:00:38]

asgerf@google.com changed reviewers:
+ sra@google.com

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

lgtm

https://codereview.chromium.org/1476293003/diff/1/pkg/compiler/lib/src/js_bac...
File pkg/compiler/lib/src/js_backend/codegen/codegen.dart (right):

https://codereview.chromium.org/1476293003/diff/1/pkg/compiler/lib/src/js_bac...
pkg/compiler/lib/src/js_backend/codegen/codegen.dart:537: if (node.next is
tree_ir.Unreachable) {
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]

https://codereview.chromium.org/1476293003/diff/1/pkg/compiler/lib/src/js_bac...
File pkg/compiler/lib/src/js_backend/codegen/codegen.dart (right):

https://codereview.chromium.org/1476293003/diff/1/pkg/compiler/lib/src/js_bac...
pkg/compiler/lib/src/js_backend/codegen/codegen.dart:537: if (node.next is
tree_ir.Unreachable) {
On 2015/11/30 18:59:40, sra1 wrote:
> 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");
>       },

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.

[sra1, 2015-12-01 21:03:21]

lgtm

[asgerf, 2015-12-02 11:40:25]

Description was changed from

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

BUG=
==========

to

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

BUG=
R=sra@google.com

Committed:
https://github.com/dart-lang/sdk/commit/91e2862a98b5aeb8bdeca729e1de60af7b853695
==========

[asgerf, 2015-12-02 11:40:26]

Committed patchset #4 (id:60001) manually as
91e2862a98b5aeb8bdeca729e1de60af7b853695 (presubmit successful).