implement kernel -> ssa for literals

implement kernel -> ssa for literals

R=sigmund@google.com

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

[Harry Terkelsen, 2016-08-26 18:01:19]

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

[Siggi Cherem (dart-lang), 2016-08-26 18:15:58]

lgtm

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

https://codereview.chromium.org/2280133002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:130: ConstantValue
getConstantForNode(ast.Node node) {
Let's create a library and a class with the ir->ast abstractions, so the code
here would contain no import to tree.dart.

For example:
class Kernel2AstAdapter {
  constantValueForSymbol(ir.SymbolLiteral node); // internally accesses node +
resolvedAst to get the right data.
}

https://codereview.chromium.org/2280133002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:193: new
ast.DartString.literal(stringLiteral.value), compiler));
for now because DartString is pretty independent of the rest of the ast, it's
less of an issue IMO to have that dependency, but we can in the future create
the constant directly from the string value (e.g. using `new
StringConstantValue.fromString`).

For now, if we move the rest to the Kernel2ASTAdapter, let's update the import
above to import `tree/dart_string.dart` instead of `tree.dart`.

[Harry Terkelsen, 2016-08-26 19:52:58]

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

https://codereview.chromium.org/2280133002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:130: ConstantValue
getConstantForNode(ast.Node node) {
On 2016/08/26 18:15:57, Siggi Cherem (dart-lang) wrote:
> Let's create a library and a class with the ir->ast abstractions, so the code
> here would contain no import to tree.dart.
> 
> For example:
> class Kernel2AstAdapter {
>   constantValueForSymbol(ir.SymbolLiteral node); // internally accesses node +
> resolvedAst to get the right data.
> }
> 

Done.

https://codereview.chromium.org/2280133002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:193: new
ast.DartString.literal(stringLiteral.value), compiler));
On 2016/08/26 18:15:57, Siggi Cherem (dart-lang) wrote:
> for now because DartString is pretty independent of the rest of the ast, it's
> less of an issue IMO to have that dependency, but we can in the future create
> the constant directly from the string value (e.g. using `new
> StringConstantValue.fromString`).
> 
> For now, if we move the rest to the Kernel2ASTAdapter, let's update the import
> above to import `tree/dart_string.dart` instead of `tree.dart`.

Done.

[Harry Terkelsen, 2016-08-26 19:53:33]

Description was changed from

==========
implement kernel -> ssa for literals
==========

to

==========
implement kernel -> ssa for literals

R=sigmund@google.com

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

[Harry Terkelsen, 2016-08-26 19:53:34]

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

[Siggi Cherem (dart-lang), 2016-08-26 20:27:11]

thanks!

https://codereview.chromium.org/2280133002/diff/20001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2280133002/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:5: import
'package:compiler/src/ssa/kernel_ast_adapter.dart';
nit: use relative import? (just to be homogeneous with the code below)

https://codereview.chromium.org/2280133002/diff/20001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart (right):

https://codereview.chromium.org/2280133002/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart:23: ConstantValue
getConstantFor(ir.Node node) {
it seems to me like you only use this for Symbol, maybe we should restrict it to
just that?

[Harry Terkelsen, 2016-08-26 20:30:21]

On 2016/08/26 20:27:11, Siggi Cherem (dart-lang) wrote:
> thanks!
> 
>
https://codereview.chromium.org/2280133002/diff/20001/pkg/compiler/lib/src/ss...
> File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):
> 
>
https://codereview.chromium.org/2280133002/diff/20001/pkg/compiler/lib/src/ss...
> pkg/compiler/lib/src/ssa/builder_kernel.dart:5: import
> 'package:compiler/src/ssa/kernel_ast_adapter.dart';
> nit: use relative import? (just to be homogeneous with the code below)
> 
>
https://codereview.chromium.org/2280133002/diff/20001/pkg/compiler/lib/src/ss...
> File pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart (right):
> 
>
https://codereview.chromium.org/2280133002/diff/20001/pkg/compiler/lib/src/ss...
> pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart:23: ConstantValue
> getConstantFor(ir.Node node) {
> it seems to me like you only use this for Symbol, maybe we should restrict it
to
> just that?

Thanks! I'll include these changes in the next kernel->ssa CL

[Siggi Cherem (dart-lang), 2016-08-26 20:45:05]

On 2016/08/26 20:30:21, Harry Terkelsen wrote:
> On 2016/08/26 20:27:11, Siggi Cherem (dart-lang) wrote:
> > thanks!
> > 
> >
>
https://codereview.chromium.org/2280133002/diff/20001/pkg/compiler/lib/src/ss...
> > File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):
> > 
> >
>
https://codereview.chromium.org/2280133002/diff/20001/pkg/compiler/lib/src/ss...
> > pkg/compiler/lib/src/ssa/builder_kernel.dart:5: import
> > 'package:compiler/src/ssa/kernel_ast_adapter.dart';
> > nit: use relative import? (just to be homogeneous with the code below)
> > 
> >
>
https://codereview.chromium.org/2280133002/diff/20001/pkg/compiler/lib/src/ss...
> > File pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart (right):
> > 
> >
>
https://codereview.chromium.org/2280133002/diff/20001/pkg/compiler/lib/src/ss...
> > pkg/compiler/lib/src/ssa/kernel_ast_adapter.dart:23: ConstantValue
> > getConstantFor(ir.Node node) {
> > it seems to me like you only use this for Symbol, maybe we should restrict
it
> to
> > just that?
> 
> Thanks! I'll include these changes in the next kernel->ssa CL

Thanks, that sounds great.

Actually I might go ahead and fix the packge:url on my next set of .status
updates I'm doing for --fast-startup.

Surprisingly, the import broke dart2js/analyze_unused_dart2js_test :-(. I think
that test loads dart2js via file:/// urls, so the import add a duplicate set of
libraries to our compiler...