Add Load/Store Static/Instance fields.

Add Load/Store Static/Instance fields.
Committed: https://code.google.com/p/dart/source/detail?r=4972

[Kevin Millikin (Google), 2012-03-05 11:02:41]

LGTM, with the first issue that is a minor bug fixed.

The other two are really something we should sort out when we get a chance and
opt for uniformity, but don't matter crucially right now.

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

https://chromiumcodereview.appspot.com/9586024/diff/1/runtime/vm/flow_graph_b...
runtime/vm/flow_graph_builder.cc:565: ValueGraphVisitor for_value(owner(),
temp_index());
Second argument should be for_instance.temp_index().

https://chromiumcodereview.appspot.com/9586024/diff/1/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler_x64.cc (right):

https://chromiumcodereview.appspot.com/9586024/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.cc:197:
GenerateAssertAssignable(comp->node_id(),
There is an AssertAssignableComp that can go in the IL instruction stream (see
EffectGraphVisitor::VisitStoreLocalNode).

The idea is that by exposing them in the IL they can be eliminated and/or
shared.

I think it's up in the air whether we insert them up front or later (but I
prefer up front a bit).  For now, it probably doesn't even matter if we're
uniform as long as we keep it in mind as an issue to sort out.

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

https://chromiumcodereview.appspot.com/9586024/diff/1/runtime/vm/intermediate...
runtime/vm/intermediate_language.h:304: RawString* name() const { return
ast_node_.field().name(); }
Might also just expose a const Field& accessor instead (also in
StoreInstanceFieldComp below).  I've been doing something like that because it's
more flexible and less API surface.

I don't have strong opinions one way or the other except that it should feel
uniform when we're all done adding classes to the IL.

[srdjan, 2012-03-05 18:35:20]

Thanks for the review.

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

https://chromiumcodereview.appspot.com/9586024/diff/1/runtime/vm/flow_graph_b...
runtime/vm/flow_graph_builder.cc:565: ValueGraphVisitor for_value(owner(),
temp_index());
On 2012/03/05 11:02:41, kmillikin wrote:
> Second argument should be for_instance.temp_index().

Done.

https://chromiumcodereview.appspot.com/9586024/diff/1/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler_x64.cc (right):

https://chromiumcodereview.appspot.com/9586024/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.cc:197:
GenerateAssertAssignable(comp->node_id(),
On 2012/03/05 11:02:41, kmillikin wrote:
> There is an AssertAssignableComp that can go in the IL instruction stream (see
> EffectGraphVisitor::VisitStoreLocalNode).
> 
> The idea is that by exposing them in the IL they can be eliminated and/or
> shared.
> 
> I think it's up in the air whether we insert them up front or later (but I
> prefer up front a bit).  For now, it probably doesn't even matter if we're
> uniform as long as we keep it in mind as an issue to sort out.

Moved to graph builder.

https://chromiumcodereview.appspot.com/9586024/diff/1/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_x64.cc:217: if (FLAG_enable_type_checks) {
Removed here as well.

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

https://chromiumcodereview.appspot.com/9586024/diff/1/runtime/vm/intermediate...
runtime/vm/intermediate_language.h:304: RawString* name() const { return
ast_node_.field().name(); }
On 2012/03/05 11:02:41, kmillikin wrote:
> Might also just expose a const Field& accessor instead (also in
> StoreInstanceFieldComp below).  I've been doing something like that because
it's
> more flexible and less API surface.
> 
> I don't have strong opinions one way or the other except that it should feel
> uniform when we're all done adding classes to the IL.

Using Field type as suggested