dart2js cps: Share interceptors by default and propagate to use later.

dart2js cps: Share interceptors by default and propagate to use later.

We now only introduce one getInterceptor call per primitive.

(The main use for GVN seemed to be cleaning up these calls, but we
might as well not introduce them in the first place).

To propagate single-used interceptors to their use, propagation of
constant-like expressions has changed.

There is a new CPS pass, let sinking, which sinks single-used pure
primitives to their use when the use is not inside a loop.

Conversely, the tree IR's assignment propagation has been made less
aggressive to compensate for things now handled by let sinking.

R=floitsch@google.com

Committed: https://github.com/dart-lang/sdk/commit/6e0a210da0d9593802451f4a359f2c4a3e64d3d4

[asgerf, 2015-07-17 15:45:13]

asgerf@google.com changed reviewers:
+ floitsch@google.com, karlklose@google.com

[floitsch, 2015-07-17 17:19:46]

LGTM.

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_tracer.dart (right):

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_tracer.dart:113: String
nameContinuation(cps_ir.Continuation cont) {
new line before and after nested functions.

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/letsinking.dart (right):

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:14: /// [LoopHierarchy].
I think it's always nice to have an example. (doesn't matter in which language:
pure Dart, CPS nodes, pseudo, ...)

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:24: loopHierarchy = new
LoopHierarchy(node);
I'm ok this way, but have a *slight* preference to calling a function to do
work.
That is: new LoopHierarchy(node)..compute()

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:124: if (node.isRecursive)
multi-line ifs should have {}

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:142: Continuation
_determineInnerLoop(Continuation c1, Continuation c2) {
The main-goal of this function is to mark nesting, and not the determining of
which one is nested. (that could be done by a simple loopDepth check).
I would turn it around:

/// Marks the innermost loop as a subloop of the other loop.
///
/// Returns the innermost loop.
///
/// Both continuations, [c1] and [c2] may be null (i.e. no loop).
///
/// A loop is said ...
Contination markNestedLoop(.... {
...
}

note: in general I try to reference the arguments in the description, so that a
good editor could highlight the corresponding dart-doc section when I write a
call to the function.

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:158: /// that can be invoked
recursively can be invoked from [cont]
-can be invoked-

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:188: // We step into call
continuations as part of the same basic block.
I don't understand this comment.

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:217: // traversal, these do not get
their loop target set by otherwise.
-by-

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/optimizers.dart (right):

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/optimizers.dart:15: export 'letsinking.dart' show
LetSinker;
let_sinking.dart

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/codegen/unsugar.dart (left):

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/codegen/unsugar.dart:247:
_glue.registerSpecializedGetInterceptor(interceptedClasses);
Was the 'register' call no necessary?

[asgerf, 2015-07-20 09:10:57]

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_tracer.dart (right):

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_tracer.dart:113: String
nameContinuation(cps_ir.Continuation cont) {
On 2015/07/17 17:19:45, floitsch wrote:
> new line before and after nested functions.

Done.

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/letsinking.dart (right):

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:14: /// [LoopHierarchy].
On 2015/07/17 17:19:45, floitsch wrote:
> I think it's always nice to have an example. (doesn't matter in which
language:
> pure Dart, CPS nodes, pseudo, ...)

Added an example based on the one from LoopHierarchy.

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:24: loopHierarchy = new
LoopHierarchy(node);
On 2015/07/17 17:19:45, floitsch wrote:
> I'm ok this way, but have a *slight* preference to calling a function to do
> work.
> That is: new LoopHierarchy(node)..compute()

Acknowledged.

I usually do that if the class is called SomethingVisitor or SomethingBuilder or
in general Something<Verb>.

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:124: if (node.isRecursive)
On 2015/07/17 17:19:45, floitsch wrote:
> multi-line ifs should have {}

Done.

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:142: Continuation
_determineInnerLoop(Continuation c1, Continuation c2) {
On 2015/07/17 17:19:45, floitsch wrote:
> The main-goal of this function is to mark nesting, and not the determining of
> which one is nested. (that could be done by a simple loopDepth check).
> I would turn it around:
> 
> /// Marks the innermost loop as a subloop of the other loop.
> ///
> /// Returns the innermost loop.
> ///
> /// Both continuations, [c1] and [c2] may be null (i.e. no loop).
> ///
> /// A loop is said ...
> Contination markNestedLoop(.... {
> ...
> }
> 
> note: in general I try to reference the arguments in the description, so that
a
> good editor could highlight the corresponding dart-doc section when I write a
> call to the function.

Done.

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:158: /// that can be invoked
recursively can be invoked from [cont]
On 2015/07/17 17:19:45, floitsch wrote:
> -can be invoked-

Done.

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:188: // We step into call
continuations as part of the same basic block.
On 2015/07/17 17:19:45, floitsch wrote:
> I don't understand this comment.

Refactored code and comment.

The "is CallExpression" branch has been merged with "is LetCont".

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/letsinking.dart:217: // traversal, these do not get
their loop target set by otherwise.
On 2015/07/17 17:19:45, floitsch wrote:
> -by-

Done.

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/optimizers.dart (right):

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/optimizers.dart:15: export 'letsinking.dart' show
LetSinker;
On 2015/07/17 17:19:46, floitsch wrote:
> let_sinking.dart

Done.

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/codegen/unsugar.dart (left):

https://codereview.chromium.org/1238163003/diff/20001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/codegen/unsugar.dart:247:
_glue.registerSpecializedGetInterceptor(interceptedClasses);
On 2015/07/17 17:19:46, floitsch wrote:
> Was the 'register' call no necessary?

No, code generation handles that. This call was a leftover from a refactoring.

[asgerf, 2015-07-20 14:53:31]

Committed patchset #4 (id:60001) manually as
6e0a210da0d9593802451f4a359f2c4a3e64d3d4 (presubmit successful).