dart2js cps: Inline single-use local closures

Inline single-use local closures.

Inlining is restricted to cases that generate opportunities for scalar replacement of aggregates.

R=asgerf@google.com

Committed: https://github.com/dart-lang/sdk/commit/0dafa33f82e47a4d35901d0e0920fb976fc3fdde

[sra1, 2015-08-27 17:40:20]

Patchset #1 (id:1) has been deleted

[sra1, 2015-08-27 17:42:42]

Patchset #1 (id:20001) has been deleted

[sra1, 2015-08-28 16:59:46]

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

[asgerf, 2015-08-31 09:17:01]

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

[asgerf, 2015-08-31 09:17:02]

DBC

https://codereview.chromium.org/1307303004/diff/60001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1307303004/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1593: ClosureClassElement
closureClassElement = receiver.classElement;
Looks like a checked-mode error if the class is an ordinary class with a 'call'
method, or even a class that where the call goes to noSuchMethod.

https://codereview.chromium.org/1307303004/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1616: if (use is SetField)
continue;
Can there actually be SetFields on the function object? Captured locals are
always boxed if they need to reassigned.

https://codereview.chromium.org/1307303004/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1621: if
(ContainsTry.analyze(target)) return false;
Why this? Is it a stack trace thing?

Even though MutableVariableEliminator bails out of catch/finally, the non-SSA'ed
MutableVariables should still be more efficient than the field lookups since
they compile to JS locals.

[sra1, 2015-08-31 16:29:06]

https://codereview.chromium.org/1307303004/diff/60001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1307303004/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1593: ClosureClassElement
closureClassElement = receiver.classElement;
On 2015/08/31 09:17:01, asgerf wrote:
> Looks like a checked-mode error if the class is an ordinary class with a
'call'
> method, or even a class that where the call goes to noSuchMethod.

I will use this as a condition - the other cases potentially have many inlining
locations so are a hazard for excessive code growth.

https://codereview.chromium.org/1307303004/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1616: if (use is SetField)
continue;
On 2015/08/31 09:17:01, asgerf wrote:
> Can there actually be SetFields on the function object? Captured locals are
> always boxed if they need to reassigned.

It is 'obvious' that we could change the closure conversion pass to push the box
cell into the closure.  This could be a simple syntactic pattern, e.g. when
there is one closure over one cell and the closure has no other free variables.

I'll leave the code but add a comment.

https://codereview.chromium.org/1307303004/diff/60001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1621: if
(ContainsTry.analyze(target)) return false;
On 2015/08/31 09:17:02, asgerf wrote:
> Why this? Is it a stack trace thing?
> 
> Even though MutableVariableEliminator bails out of catch/finally, the
non-SSA'ed
> MutableVariables should still be more efficient than the field lookups since
> they compile to JS locals.

I'll add a comment.
Most JS engines refuse to optimize functions that contain try-xxx, causing the
surrounding code to get slower.

[asgerf, 2015-08-31 17:25:14]

Also LGTM

[sra1, 2015-09-02 00:29:01]

Committed patchset #4 (id:100001) manually as
0dafa33f82e47a4d35901d0e0920fb976fc3fdde (presubmit successful).