Refactor the collection of initializer list types

Refactor the collection of initializer list types

Instead of storing a boolean to indicate whether a field is only
intialized to an integer store the propagated type instead. If
different types are encountered for the same field indicate this with
HType.UNKNOWN.

Also fix a bug in the checking, as potential single type initializers
is not the same as known single type initializers.

This is in preparation for information on other types than integers.

R=floitsch@google.com

BUG=
TEST=

Committed: https://code.google.com/p/dart/source/detail?r=8963

[Søren Gjesse, 2012-06-19 14:38:18]

PTAL

As discussed offline moved the support data structures to the backend object.

I still have not changed the keys to Element (FieldElement).

[karlklose, 2012-06-19 15:07:39]

https://chromiumcodereview.appspot.com/10584009/diff/3001/lib/compiler/implem...
File lib/compiler/implementation/compiler.dart (right):

https://chromiumcodereview.appspot.com/10584009/diff/3001/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:37: final Map<Element,
Map<SourceString, HType>> fieldInitializers;
Why not move these datastructures to the JavaScriptBackend? The HTypes are JS
specific.

[Søren Gjesse, 2012-06-20 09:07:10]

PTAL

Moved to JavaScriptBackend used Element as key.

[floitsch, 2012-06-20 10:20:33]

LGTM with comments.

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
File lib/compiler/implementation/compiler.dart (right):

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:54: class JavaScriptBackend extends
Backend {
We should probably move the class out of the compiler file.
But in a different CL.

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:119: if (fields[field] !=
propagatedType) fields[field] = HType.UNKNOWN;
can't you just union the types?

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:161: bool
couldHaveFieldOnlyIntegerSetters(Type type, Element field) {
Doesn't the  field element contain the enclosing class? If yes it should be
enough to pass the field-element.

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:170: bool
hasFieldOnlyIntegerSetters(Type type, Element field) {
I would remove this method. It is identical to the
couldHaveFieldOnlyIntegerSetters.
Also it is misleading. Even when returning true the field might still have
non-integer setters (if there are dynamic setters). So from this point of view
"couldHaveFieldOnly..." fills the role. I would also add a comment to couldHave
stating that it returns true if all seen (typed) setters have been integers.

[karlklose, 2012-06-20 11:26:32]

LGTM.

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
File lib/compiler/implementation/ssa/optimize.dart (right):

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:1204: type, left.element) &&
Does this fit in in previous line now?

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:1213: type, left.element) &&
ditto.

[Søren Gjesse, 2012-06-21 08:51:34]

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
File lib/compiler/implementation/compiler.dart (right):

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:54: class JavaScriptBackend extends
Backend {
On 2012/06/20 10:20:33, floitsch wrote:
> We should probably move the class out of the compiler file.
> But in a different CL.

Will do.

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:119: if (fields[field] !=
propagatedType) fields[field] = HType.UNKNOWN;
On 2012/06/20 10:20:33, floitsch wrote:
> can't you just union the types?

Done, that should also be better.

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:161: bool
couldHaveFieldOnlyIntegerSetters(Type type, Element field) {
On 2012/06/20 10:20:33, floitsch wrote:
> Doesn't the  field element contain the enclosing class? If yes it should be
> enough to pass the field-element.

It does. The element of the type can be a sub-class of the containing element
for the field, but that should be OK as we only use HFieldGet/Set when the field
name is unique in the class hierarchy.

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:170: bool
hasFieldOnlyIntegerSetters(Type type, Element field) {
On 2012/06/20 10:20:33, floitsch wrote:
> I would remove this method. It is identical to the
> couldHaveFieldOnlyIntegerSetters.
> Also it is misleading. Even when returning true the field might still have
> non-integer setters (if there are dynamic setters). So from this point of view
> "couldHaveFieldOnly..." fills the role. I would also add a comment to
couldHave
> stating that it returns true if all seen (typed) setters have been integers.

Changed the name to onlyFieldIntegerSettersSoFar (and dropped the other). Added
a comment. The name was supposed to indicate that the information is from
HFieldSet instructions only and does not include dynamic setters.

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
File lib/compiler/implementation/ssa/optimize.dart (right):

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:1204: type, left.element) &&
On 2012/06/20 11:26:32, karlklose wrote:
> Does this fit in in previous line now?

Done.

https://chromiumcodereview.appspot.com/10584009/diff/9001/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:1213: type, left.element) &&
On 2012/06/20 11:26:32, karlklose wrote:
> ditto.

Done.