dart2js: move type-mask intermediate data from tree-elements into the global

dart2js: move type-mask intermediate data from tree-elements into the global
inference engine.

R=johnniwinther@google.com

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

[Siggi Cherem (dart-lang), 2016-11-02 23:38:52]

Patchset #1 (id:1) has been deleted

[Siggi Cherem (dart-lang), 2016-11-03 01:03:37]

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

[Siggi Cherem (dart-lang), 2016-11-03 01:03:38]

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/ty...
File pkg/compiler/lib/src/types/types.dart (right):

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/ty...
pkg/compiler/lib/src/types/types.dart:112: class GlobalTypeInferenceElementData
{
I expect this will move under the inferrer, but I'm waiting to have a better
picture of how the inferrer will be refactored before I do that.

[Johnni Winther, 2016-11-03 08:50:22]

lgtm

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/in...
File pkg/compiler/lib/src/inferrer/simple_types_inferrer.dart (right):

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/simple_types_inferrer.dart:706: TypeMask
getterMask = inTreeData?.typeOfGetter(node);
Store the value of [inTreeData] in a variable to avoid performing the lookup
`inferrer.inTreeData` three times for the same result.

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/simple_types_inferrer.dart:820: TypeMask
getterMask = inTreeData?.typeOfGetter(node);
Ditto

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/simple_types_inferrer.dart:921: TypeMask
getterMask = inTreeData?.typeOfGetter(node);
Ditto

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/simple_types_inferrer.dart:1282: TypeMask
getterMask = inTreeData?.typeOfGetter(node);
Ditto

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/simple_types_inferrer.dart:2228: TypeMask
currentMask = inTreeData?.typeOfIteratorCurrent(node);
Ditto

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/simple_types_inferrer.dart:2246: TypeMask
iteratorMask = inTreeData?.typeOfIterator(node);
Ditto

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolution/tree_elements.dart (left):

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/tree_elements.dart:13: import
'../types/types.dart' show TypeMask;
Nice!!

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

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder.dart:6653: isCalledOnce(function),
Reinsert the transfer of the `allInlinedFunctionsCalledOnce` value here?

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/ty...
File pkg/compiler/lib/src/types/types.dart (right):

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/ty...
pkg/compiler/lib/src/types/types.dart:26: /// can prove to be correct for all
executions of the program.
Add `Thus the trivial inference will return false on its boolean properties
(giving no guarantees) and the `subclass of Object or null` type mask for the
type based queries (the runtime value could be anything)`.

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/ty...
pkg/compiler/lib/src/types/types.dart:168: 
Remove extra line?

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/ty...
pkg/compiler/lib/src/types/types.dart:188: GlobalTypeInferenceElementResult
resultOf(AstElement element) {
Add a TODO to (at some point in the future) fail if no data is available when
queried from SSA.

[Siggi Cherem (dart-lang), 2016-11-03 21:12:15]

Thanks Johnni!

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/in...
File pkg/compiler/lib/src/inferrer/simple_types_inferrer.dart (right):

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/simple_types_inferrer.dart:706: TypeMask
getterMask = inTreeData?.typeOfGetter(node);
On 2016/11/03 08:50:21, Johnni Winther wrote:
> Store the value of [inTreeData] in a variable to avoid performing the lookup
> `inferrer.inTreeData` three times for the same result.

Good point - I went a step further and made this into a final field and
initialize it upfront. This still loads a field multiple times, but it's now a
final field and not a complex getter.

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/simple_types_inferrer.dart:820: TypeMask
getterMask = inTreeData?.typeOfGetter(node);
On 2016/11/03 08:50:21, Johnni Winther wrote:
> Ditto

same here and below

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

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder.dart:6653: isCalledOnce(function),
On 2016/11/03 08:50:21, Johnni Winther wrote:
> Reinsert the transfer of the `allInlinedFunctionsCalledOnce` value here?

Note that I replaced "isFunctionCalledOnce" with "isCalledOnce". The
implementation of isCalledOnce is calling both allInlinedFunctionsCalledOnce and
isFunctionCalledOnce internally.

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/ty...
File pkg/compiler/lib/src/types/types.dart (right):

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/ty...
pkg/compiler/lib/src/types/types.dart:26: /// can prove to be correct for all
executions of the program.
On 2016/11/03 08:50:21, Johnni Winther wrote:
> Add `Thus the trivial inference will return false on its boolean properties
> (giving no guarantees) and the `subclass of Object or null` type mask for the
> type based queries (the runtime value could be anything)`.

Done.

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/ty...
pkg/compiler/lib/src/types/types.dart:168: 
On 2016/11/03 08:50:21, Johnni Winther wrote:
> Remove extra line?

Done.

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/ty...
pkg/compiler/lib/src/types/types.dart:188: GlobalTypeInferenceElementResult
resultOf(AstElement element) {
On 2016/11/03 08:50:21, Johnni Winther wrote:
> Add a TODO to (at some point in the future) fail if no data is available when
> queried from SSA.

Done.

[Johnni Winther, 2016-11-04 07:30:52]

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

https://codereview.chromium.org/2468303003/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder.dart:6653: isCalledOnce(function),
On 2016/11/03 21:12:15, Siggi Cherem (dart-lang) wrote:
> On 2016/11/03 08:50:21, Johnni Winther wrote:
> > Reinsert the transfer of the `allInlinedFunctionsCalledOnce` value here?
> 
> Note that I replaced "isFunctionCalledOnce" with "isCalledOnce". The
> implementation of isCalledOnce is calling both allInlinedFunctionsCalledOnce
and
> isFunctionCalledOnce internally.

Acknowledged.

[Siggi Cherem (dart-lang), 2016-11-08 20:55:21]

Patchset #3 (id:60001) has been deleted

[Siggi Cherem (dart-lang), 2016-11-08 20:58:23]

PTAL

Johnni - after trying to run some benchmarks to validate the CL had no semantic
changes, I discovered that the builder had incorrect data within closures and
constructor-body elements. I uploaded some fixes for that. Can you take a look
at patch #2?

[Johnni Winther, 2016-11-09 11:42:18]

lgtm

[Siggi Cherem (dart-lang), 2016-11-09 15:26:19]

Description was changed from

==========
dart2js: move type-mask intermediate data from tree-elements into the global
inference engine.
==========

to

==========
dart2js: move type-mask intermediate data from tree-elements into the global
inference engine.

R=johnniwinther@google.com

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

[Siggi Cherem (dart-lang), 2016-11-09 15:26:20]

Committed patchset #3 (id:80001) manually as
053fa1c5006dd874e04cd2248005f05b191948c2 (presubmit successful).