First shot at static type propagation and type test elimination.

First shot at static type propagation and type test elimination.
Committed: https://code.google.com/p/dart/source/detail?r=7708

[srdjan, 2012-05-16 20:16:15]

LGTM with a couple of suggestions and questions.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_builder.cc (right):

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.cc:159: context_value, Context::parent_offset(),
Type::ZoneHandle()));
Why not pass dynamic type (meaning unknown) instead of null?

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.cc:164: context_value,
Context::variable_offset(local.index()), value);
Needs a type as well, I think.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.cc:182: context_value, Context::parent_offset(),
Type::ZoneHandle()));
dynamic type instead of null?

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language.cc (right):

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:315: return Type::BoolInterface();
Bool class instead of interface type? It probably does not matter since the bool
interface/class is a 1:1 relationship.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:320: UNREACHABLE();
Maybe it would be better to remove UNREACHABLE's and return DynamicType instead?

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:337: return Type::DynamicType();
In the context of static type check elimination a dynamic type means an unknown
type, i.e., it can be anything.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:364: return value()->StaticType();
If value()->StaticType() is Dynamic (unknown), then you could return
local().type().raw() instead.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:379: // The result type of the native
function is not known.
It could be probable computed, as it is known as compile time.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:407: return value()->StaticType();
And if value's type is unknown, return field's type.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:417: return value()->StaticType();
And if value's type is unknown, return field's type.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:455: return Type::VoidType();
For each UNREACHABLE, shouldn't return be DynamicType (unknown), in case we have
to enable it?

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language.h (right):

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:91: // Static type propagation support.
Maybe instead: "Static type of the computation".

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:932: const AbstractType& type)
Can type be null or can you assert !type.IsNull() ? Please assert that it is a
ZoneObject.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1463: return computation()->StaticType();
When would you need the static type of a DoInstr?

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1544: return Type::VoidType();
Maybe move this into Instructions, so that you do not need to implement it in
every subclass. The only class who needs to really implement it is the
BindInstr, I  think.

[regis, 2012-05-16 23:20:37]

Thanks!

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_builder.cc (right):

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.cc:159: context_value, Context::parent_offset(),
Type::ZoneHandle()));
On 2012/05/16 20:16:15, srdjan wrote:
> Why not pass dynamic type (meaning unknown) instead of null?

I want to catch cases where the static type is requested by mistake (similar to
UNREACHABLE()). Using Dynamic would hide these cases.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.cc:164: context_value,
Context::variable_offset(local.index()), value);
On 2012/05/16 20:16:15, srdjan wrote:
> Needs a type as well, I think.

No, the type of interest is the type of the assigned value, which may be more
specific than the type of the field.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.cc:182: context_value, Context::parent_offset(),
Type::ZoneHandle()));
On 2012/05/16 20:16:15, srdjan wrote:
> dynamic type instead of null?

Same as above.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language.cc (right):

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:315: return Type::BoolInterface();
On 2012/05/16 20:16:15, srdjan wrote:
> Bool class instead of interface type? It probably does not matter since the
bool
> interface/class is a 1:1 relationship.

Yes, I think it does not matter.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:320: UNREACHABLE();
On 2012/05/16 20:16:15, srdjan wrote:
> Maybe it would be better to remove UNREACHABLE's and return DynamicType
instead?

As we discussed, I am now returning AbstractType::null()

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:337: return Type::DynamicType();
On 2012/05/16 20:16:15, srdjan wrote:
> In the context of static type check elimination a dynamic type means an
unknown
> type, i.e., it can be anything.

Correct.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:364: return value()->StaticType();
On 2012/05/16 20:16:15, srdjan wrote:
> If value()->StaticType() is Dynamic (unknown), then you could return
> local().type().raw() instead.

Done.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:379: // The result type of the native
function is not known.
On 2012/05/16 20:16:15, srdjan wrote:
> It could be probable computed, as it is known as compile time.

It is actually the result type of the enclosing native Dart function. Can we
trust the native call? Added TODO.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:407: return value()->StaticType();
On 2012/05/16 20:16:15, srdjan wrote:
> And if value's type is unknown, return field's type.

Done.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:417: return value()->StaticType();
On 2012/05/16 20:16:15, srdjan wrote:
> And if value's type is unknown, return field's type.

Done.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:455: return Type::VoidType();
On 2012/05/16 20:16:15, srdjan wrote:
> For each UNREACHABLE, shouldn't return be DynamicType (unknown), in case we
have
> to enable it?

As explained above, I want to catch wrong requests for static type. I am now
using null instead of void.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language.h (right):

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:91: // Static type propagation support.
On 2012/05/16 20:16:15, srdjan wrote:
> Maybe instead: "Static type of the computation".

Done.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:932: const AbstractType& type)
On 2012/05/16 20:16:15, srdjan wrote:
> Can type be null or can you assert !type.IsNull() ? Please assert that it is a
> ZoneObject.

Added assert for ZoneHandle and comment about allowed to be null. Removed
comment below now redundant.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1463: return computation()->StaticType();
On 2012/05/16 20:16:15, srdjan wrote:
> When would you need the static type of a DoInstr?

You are right. Never needed. Changed to UNREACHABLE.

https://chromiumcodereview.appspot.com/10399051/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1544: return Type::VoidType();
On 2012/05/16 20:16:15, srdjan wrote:
> Maybe move this into Instructions, so that you do not need to implement it in
> every subclass. The only class who needs to really implement it is the
> BindInstr, I  think.

Good point. Done.