Build entire program with kernel for conversion to ssa.

Build entire program with kernel for conversion to ssa.

This CL has the kernel task of the backend building the entire program.
This causes the compiler to resolve a lot more than usual, and so we need
to support many language features in the kernel -> ssa builder to compile
all of the helper functions that are included in the output.

R=sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/8be34ddd395e742ee6e9a77e405cc373adcb0040

[Harry Terkelsen, 2016-09-13 23:08:29]

het@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2016-09-13 23:48:20]

lgtm, only minor nits below.

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/kernel_visitor.dart (right):

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/kernel_visitor.dart:1272:
associateNode(compound.value, node);
in this case, should both compound and compound.value have the associated node?
or should it only be the .value?

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/ssa/bu...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:283: Element element =
astAdapter.getElement(staticGet.target);
nit: consider adding a variable for target
  Member target = staticGet.target;
  ...

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:286: ir.Procedure target =
staticGet.target;
(with a variable above, type promotion would work and you'll be able to get rid
of this one here)

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:292: push(new HStatic(element,
astAdapter.inferredTypeOf(staticGet.target)));
nit: maybe add a comment that this is a tear-off?

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:295: push(new HStatic(element,
astAdapter.inferredTypeOf(staticGet.target)));
A couple questions here:
(a) shouldn't this element also be element.declaration? If so, could we combine
with the other branch above? For example:

Member target = ...;
if (target is ir.Procedure && target.kind == Getter) {
  ...
  return;
}

push(new HStatic(element.declaration, ...));


(b) are there more cases to handle here? IIUC this is static fields right now,
do we need to also include details about static-const-get and lazy statics here?

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:442: ir.Class
containingClass(ir.TreeNode node) {
nit => make static and private?

[Harry Terkelsen, 2016-09-14 00:07:24]

Thanks, Siggi!

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/kernel_visitor.dart (right):

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/kernel_visitor.dart:1272:
associateNode(compound.value, node);
On 2016/09/13 23:48:19, Siggi Cherem (dart-lang) wrote:
> in this case, should both compound and compound.value have the associated
node?
> or should it only be the .value?
> 

I think you're right, if it is a compound assignment then only the value should
be associated with the node

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/ssa/bu...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:283: Element element =
astAdapter.getElement(staticGet.target);
On 2016/09/13 23:48:19, Siggi Cherem (dart-lang) wrote:
> nit: consider adding a variable for target
>   Member target = staticGet.target;
>   ...

Done.

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:286: ir.Procedure target =
staticGet.target;
On 2016/09/13 23:48:19, Siggi Cherem (dart-lang) wrote:
> (with a variable above, type promotion would work and you'll be able to get
rid
> of this one here)

Done.

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:292: push(new HStatic(element,
astAdapter.inferredTypeOf(staticGet.target)));
On 2016/09/13 23:48:19, Siggi Cherem (dart-lang) wrote:
> nit: maybe add a comment that this is a tear-off?

Done.

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:295: push(new HStatic(element,
astAdapter.inferredTypeOf(staticGet.target)));
On 2016/09/13 23:48:19, Siggi Cherem (dart-lang) wrote:
> A couple questions here:
> (a) shouldn't this element also be element.declaration? If so, could we
combine
> with the other branch above? For example:
> 
> Member target = ...;
> if (target is ir.Procedure && target.kind == Getter) {
>   ...
>   return;
> }
> 
> push(new HStatic(element.declaration, ...));
> 
> 
> (b) are there more cases to handle here? IIUC this is static fields right now,
> do we need to also include details about static-const-get and lazy statics
here?

a) Yes. Done

b) Yes, there are more cases that need to be handled, this was just the minimal
functionality needed to get the tests passing

https://codereview.chromium.org/2338093002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:442: ir.Class
containingClass(ir.TreeNode node) {
On 2016/09/13 23:48:19, Siggi Cherem (dart-lang) wrote:
> nit => make static and private?

Done.

[Harry Terkelsen, 2016-09-14 16:42:27]

Description was changed from

==========
Build entire program with kernel for conversion to ssa.

This CL has the kernel task of the backend building the entire program.
This causes the compiler to resolve a lot more than usual, and so we need
to support many language features in the kernel -> ssa builder to compile
all of the helper functions that are included in the output.
==========

to

==========
Build entire program with kernel for conversion to ssa.

This CL has the kernel task of the backend building the entire program.
This causes the compiler to resolve a lot more than usual, and so we need
to support many language features in the kernel -> ssa builder to compile
all of the helper functions that are included in the output.

R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/8be34ddd395e742ee6e9a77e405cc373adcb0040
==========

[Harry Terkelsen, 2016-09-14 16:42:28]

Committed patchset #2 (id:20001) manually as
8be34ddd395e742ee6e9a77e405cc373adcb0040 (presubmit successful).