Some cleanups, started implementing checked instance calls, better equality operation.

Some cleanups, started implementing checked instance calls, better equality operation.
Committed: https://code.google.com/p/dart/source/detail?r=8626

[Vyacheslav Egorov (Google), 2012-06-13 09:02:26]

lgtm

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/flow_gra...
File runtime/vm/flow_graph_optimizer.cc (right):

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/flow_gra...
runtime/vm/flow_graph_optimizer.cc:325: ExtractClassIdsAndTargets(ic_data,
result, dummy);
how about passing NULL instead of dummy? Collecting stuff that is not used is a
bit wasteful.

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/flow_gra...
runtime/vm/flow_graph_optimizer.cc:434: Token::Kind op_kind =
Token::GetBinaryOp(function_name);
Can we just store op_kind in the InstanceCallComp produced from binary/unary
ops? Parsing it seems so strange when builder actually knew what kind of token
was there.

The same for getter (we can mark them with kGet token)

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
File runtime/vm/intermediate_language.cc (right):

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
runtime/vm/intermediate_language.cc:62: other->set_instr(other->instr());
we definitely need to eradicate bind/do. the fact that computation knows
instruction and instruction knows computation just tells us that they are the
very same thing.

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
File runtime/vm/intermediate_language.h (right):

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
runtime/vm/intermediate_language.h:503: class CheckedInstanceCallComp : public
Computation {
Should not this be PolymorphicInstanceCall or something similar?

When I see 'Checked' I think 'checked mode' while in-fact it seems to be
fast-path probes/direct-calls for polymorphic call site.

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
File runtime/vm/intermediate_language_ia32.cc (right):

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
runtime/vm/intermediate_language_ia32.cc:210: Register left =
locs()->in(0).reg();
I think when instruction code is large enough it makes sense to split its
generation into different functions for readability.

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
File runtime/vm/intermediate_language_x64.cc (right):

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
runtime/vm/intermediate_language_x64.cc:286: Register left =
locs()->in(0).reg();
Please remove dead code.

[srdjan, 2012-06-13 18:34:23]

Thanks, one comment will be dealt with in an upcoming CL.

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/flow_gra...
File runtime/vm/flow_graph_optimizer.cc (right):

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/flow_gra...
runtime/vm/flow_graph_optimizer.cc:325: ExtractClassIdsAndTargets(ic_data,
result, dummy);
On 2012/06/13 09:02:27, Vyacheslav Egorov (Google) wrote:
> how about passing NULL instead of dummy? Collecting stuff that is not used is
a
> bit wasteful.

Done.

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/flow_gra...
runtime/vm/flow_graph_optimizer.cc:434: Token::Kind op_kind =
Token::GetBinaryOp(function_name);
On 2012/06/13 09:02:27, Vyacheslav Egorov (Google) wrote:
> Can we just store op_kind in the InstanceCallComp produced from binary/unary
> ops? Parsing it seems so strange when builder actually knew what kind of token
> was there.
> 
> The same for getter (we can mark them with kGet token)

Yes, next CL.

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
File runtime/vm/intermediate_language.cc (right):

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
runtime/vm/intermediate_language.cc:62: other->set_instr(other->instr());
On 2012/06/13 09:02:27, Vyacheslav Egorov (Google) wrote:
> we definitely need to eradicate bind/do. the fact that computation knows
> instruction and instruction knows computation just tells us that they are the
> very same thing.

Yes, agreed.

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
File runtime/vm/intermediate_language.h (right):

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
runtime/vm/intermediate_language.h:503: class CheckedInstanceCallComp : public
Computation {
On 2012/06/13 09:02:27, Vyacheslav Egorov (Google) wrote:
> Should not this be PolymorphicInstanceCall or something similar?
> 
> When I see 'Checked' I think 'checked mode' while in-fact it seems to be
> fast-path probes/direct-calls for polymorphic call site.

Renaming to PolymorphicInstanceCall.

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
File runtime/vm/intermediate_language_ia32.cc (right):

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
runtime/vm/intermediate_language_ia32.cc:210: Register left =
locs()->in(0).reg();
On 2012/06/13 09:02:27, Vyacheslav Egorov (Google) wrote:
> I think when instruction code is large enough it makes sense to split its
> generation into different functions for readability.

Done.

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
File runtime/vm/intermediate_language_x64.cc (right):

https://chromiumcodereview.appspot.com/10541135/diff/9002/runtime/vm/intermed...
runtime/vm/intermediate_language_x64.cc:286: Register left =
locs()->in(0).reg();
On 2012/06/13 09:02:27, Vyacheslav Egorov (Google) wrote:
> Please remove dead code.

Done.