dart2js cps: LICM, GVN, and constify for interceptors in one pass.

dart2js cps: LICM, GVN, and constify for interceptors in one pass.

Neither of these optimizations can "obviously" be placed before any of
the others without missing out on an optimization:

Constifying an interceptor makes it LICM'able because its dependency on
the input disappears. This suggests we should constify before LICM.

Interceptors in disjoint scopes might become sharable after LICM.
This suggests we LICM before sharing.

Constifying an interceptor can prevent sharing, because we "forget"
which input it came from. This suggests we share before constifying.

The last point could also be resolved by performing a stronger analysis
to determine which interceptors in scope are a valid substitute for a
given interceptor. But having everything in one pass isn't too bad.

BUG=
R=sra@google.com

Committed: https://github.com/dart-lang/sdk/commit/3fa9c1f80abefbfedf1e231bf2adf32a81d8e826

[asgerf, 2015-09-24 09:03:27]

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

[asgerf, 2015-09-24 09:03:28]

FYI the diff for share_interceptors.dart is horrible, as it has very little in
common with the old pass of the same name.

[sra1, 2015-09-25 18:26:17]

LGTM

I still feel that we are missing a level of editing functions to move or remove
nodes.

https://chromiumcodereview.appspot.com/1364703004/diff/1/pkg/compiler/lib/src...
File pkg/compiler/lib/src/cps_ir/share_interceptors.dart (right):

https://chromiumcodereview.appspot.com/1364703004/diff/1/pkg/compiler/lib/src...
pkg/compiler/lib/src/cps_ir/share_interceptors.dart:100:
interceptor.input.unlink();
Perhaps we should add a more general dead node removal and use that. e.g.

    node.remove();

The method would assert that the node was correctly linked, i.e.

   node.hasNoUses
   node.parent.body == node
   node.body.parent == node

https://chromiumcodereview.appspot.com/1364703004/diff/1/pkg/compiler/lib/src...
pkg/compiler/lib/src/cps_ir/share_interceptors.dart:115:
interceptor.input.unlink();
These five lines are the same as above.
If we don't get a general dead node removal method, maybe have a local method


   return replaceWith(existing);

[asgerf, 2015-09-28 10:50:14]

https://chromiumcodereview.appspot.com/1364703004/diff/1/pkg/compiler/lib/src...
File pkg/compiler/lib/src/cps_ir/share_interceptors.dart (right):

https://chromiumcodereview.appspot.com/1364703004/diff/1/pkg/compiler/lib/src...
pkg/compiler/lib/src/cps_ir/share_interceptors.dart:100:
interceptor.input.unlink();
On 2015/09/25 18:26:17, sra1 wrote:
> Perhaps we should add a more general dead node removal and use that. e.g.
> 
>     node.remove();
> 
> The method would assert that the node was correctly linked, i.e.
> 
>    node.hasNoUses
>    node.parent.body == node
>    node.body.parent == node

I was reluctant to do this, but I admit it does make the code a lot more
readable.

Since this affects a bunch of other passes I prefer to do this in another CL and
get a separate review of that.

https://chromiumcodereview.appspot.com/1364703004/diff/1/pkg/compiler/lib/src...
pkg/compiler/lib/src/cps_ir/share_interceptors.dart:115:
interceptor.input.unlink();
On 2015/09/25 18:26:17, sra1 wrote:
> These five lines are the same as above.
> If we don't get a general dead node removal method, maybe have a local method
> 
> 
>    return replaceWith(existing);

(see above)

[asgerf, 2015-09-28 11:55:27]

Committed patchset #1 (id:1) manually as
3fa9c1f80abefbfedf1e231bf2adf32a81d8e826 (presubmit successful).