Move types out of the HInstructions.

Move types out of the HInstructions.

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

[floitsch, 2012-08-06 15:42:40]

Motivation:
- while compiling the bailout method we sometimes want to know how a specific
instruction would behave in optimized mode. This is dependent on the type.
- in theory we could avoid recomputing the types several times (for example when
switching from bailout to optimized mode).
- we can think of more fine grained type-analysis and get rid of type-conversion
nodes. If we have a different type-set at each instruction we could catch cases
like:
var o = toto();
o.bar();  // after this instruction o is known not to be null.
o.foo();  // o is known to be of a type that has both bar and foo.
o.gee;    // we can potentially generate better code here.

In this case we would have a separate type-set at each instruction.

Discussion points:
- I took the work-item as container for information that is only alive for one
element (contrary to the backend which stays alive during the compilation of all
elements).
- Currently the type-container is a wrapper around a hashmap. This is slow, but
currently acceptable:
  * -12% in the worst case for a compilation of the compiler itself.
  * testing-time goes from 14:36 to 15:02.
  We will of course need to optimize this.

[Lasse Reichstein Nielsen, 2012-08-08 07:44:53]

LGTM.
Generally consider reducing the dependencies, so you only depend on a HTypeMap
instead of a JavaScriptWorkItem, if that is all you use of the work item.

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/enqueue.dart (right):

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:91:
queue.add(backend.createWorkItem(element, elements));
If you only use 'backend' for 'createWorkItem', just store the function instead
of passing the entire backend and making the Enquerer depend on Backend
unnecessarily.

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/ssa/bailout.dart (right):

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/bailout.dart:185: // Start by reverting the
propagated type. If we add a type guard then the
How do you revert a type?
"propagated" isn't mentioned later, so what is its relevance here?

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/bailout.dart:187: // then this avoids subsequent
instructions to use the the wrong type.
"avoids ... to use" -> "avoids ... using"
"the the" -> "the"

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/builder.dart:141: final JavaScriptBackend
backend;
Is this used?

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/ssa/nodes.dart (left):

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/nodes.dart:796: HType propagatedType =
HType.UNKNOWN;
Ick, what was that doing down here. :)

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/ssa/nodes.dart (right):

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/nodes.dart:1409: final bool _isStatement;
Fields at the top, even if private.

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

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:639: final JavaScriptBackend
backend;
Used?

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:852: final JavaScriptWorkItem
work;
Do you need the work item, or do you just need the types?
If so, just pass in the types directly (unless the types object on the work-item
can change over time, ofcourse)

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:1193: final JavaScriptWorkItem
work;
Just needed for types?

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/ssa/types_propagation.dart (right):

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/types_propagation.dart:15:
SsaTypePropagator(this.compiler, JavaScriptWorkItem work)
Consider just passing in the HTypeMap.

[floitsch, 2012-08-08 19:18:37]

PTAL.
I don't really like the ItemCompilationContext name, but that's easy to change.
For now I just pass the types to all passes that need them. Eventually we will
probably need to pass the full context, so that we can use two different
type-maps at the same time (which is the whole motivation behind this CL).

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/enqueue.dart (right):

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:91:
queue.add(backend.createWorkItem(element, elements));
On 2012/08/08 07:44:53, Lasse Reichstein Nielsen wrote:
> If you only use 'backend' for 'createWorkItem', just store the function
instead
> of passing the entire backend and making the Enquerer depend on Backend
> unnecessarily.

Done.

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/ssa/bailout.dart (right):

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/bailout.dart:185: // Start by reverting the
propagated type. If we add a type guard then the
On 2012/08/08 07:44:53, Lasse Reichstein Nielsen wrote:
> How do you revert a type?
> "propagated" isn't mentioned later, so what is its relevance here?

Done.

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/bailout.dart:187: // then this avoids subsequent
instructions to use the the wrong type.
On 2012/08/08 07:44:53, Lasse Reichstein Nielsen wrote:
> "avoids ... to use" -> "avoids ... using"
> "the the" -> "the"
reworded.

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/builder.dart:141: final JavaScriptBackend
backend;
On 2012/08/08 07:44:53, Lasse Reichstein Nielsen wrote:
> Is this used?

Done.

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/ssa/nodes.dart (right):

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/nodes.dart:1409: final bool _isStatement;
On 2012/08/08 07:44:53, Lasse Reichstein Nielsen wrote:
> Fields at the top, even if private.

Done.

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

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:639: final JavaScriptBackend
backend;
On 2012/08/08 07:44:53, Lasse Reichstein Nielsen wrote:
> Used?

Done.

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:852: final JavaScriptWorkItem
work;
On 2012/08/08 07:44:53, Lasse Reichstein Nielsen wrote:
> Do you need the work item, or do you just need the types?
> If so, just pass in the types directly (unless the types object on the
work-item
> can change over time, ofcourse)

Done.

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:1193: final JavaScriptWorkItem
work;
On 2012/08/08 07:44:53, Lasse Reichstein Nielsen wrote:
> Just needed for types?

Done.

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/ssa/types_propagation.dart (right):

https://chromiumcodereview.appspot.com/10827180/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/types_propagation.dart:15:
SsaTypePropagator(this.compiler, JavaScriptWorkItem work)
On 2012/08/08 07:44:53, Lasse Reichstein Nielsen wrote:
> Consider just passing in the HTypeMap.

Done.

[Lasse Reichstein Nielsen, 2012-08-16 10:41:34]

LGTM

https://chromiumcodereview.appspot.com/10827180/diff/3030/lib/compiler/implem...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10827180/diff/3030/lib/compiler/implem...
lib/compiler/implementation/ssa/codegen.dart:253: SsaCodeGenerator(this.backend,
Why is the constructor down here. Move it up just after the fields.

https://chromiumcodereview.appspot.com/10827180/diff/3030/lib/compiler/implem...
lib/compiler/implementation/ssa/codegen.dart:259: (work.compilationContext as
JavaScriptItemCompilationContext).types,
Why not just expect a JavaScriptWorkItem if you know it's one? 
Don't use 'as' if you don't need a dynamic check.

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

https://chromiumcodereview.appspot.com/10827180/diff/3030/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:29: void optimize(WorkItem work,
HGraph graph) {
You know this work item is a JavaScriptWorkItem?

[ngeoffray, 2012-08-16 13:07:00]

LGTM

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

https://chromiumcodereview.appspot.com/10827180/diff/3030/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:70: : types = new HTypeMap();
fits in one line

[floitsch, 2012-08-16 14:10:04]

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

https://chromiumcodereview.appspot.com/10827180/diff/3030/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:70: : types = new HTypeMap();
On 2012/08/16 13:07:00, ngeoffray wrote:
> fits in one line

Done.

https://chromiumcodereview.appspot.com/10827180/diff/3030/lib/compiler/implem...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10827180/diff/3030/lib/compiler/implem...
lib/compiler/implementation/ssa/codegen.dart:253: SsaCodeGenerator(this.backend,
On 2012/08/16 10:41:34, Lasse Reichstein Nielsen wrote:
> Why is the constructor down here. Move it up just after the fields.

Done.

https://chromiumcodereview.appspot.com/10827180/diff/3030/lib/compiler/implem...
lib/compiler/implementation/ssa/codegen.dart:259: (work.compilationContext as
JavaScriptItemCompilationContext).types,
On 2012/08/16 10:41:34, Lasse Reichstein Nielsen wrote:
> Why not just expect a JavaScriptWorkItem if you know it's one? 
> Don't use 'as' if you don't need a dynamic check. 

There is no JavaScriptWorkItem anymore.
Keeping as is, but I can change it in a future CL.

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

https://chromiumcodereview.appspot.com/10827180/diff/3030/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:29: void optimize(WorkItem work,
HGraph graph) {
On 2012/08/16 10:41:34, Lasse Reichstein Nielsen wrote:
> You know this work item is a JavaScriptWorkItem?

There is no JavaScriptWorkItem anymore.