Split array loads/stores for growable arrays into two IL instructions.

Split array loads/stores for growable arrays into two IL instructions.

This simplifies the code generation function for indexed loads/stores
a lot and help eliminating the load of the backing store in the future.

Enable CSE for loads of immutable fields like string length or fixed array
length.

Rename HasSideEffect into AffectedBySideEffect. Currently, only instructions
that are not affected by side effects can participate in hoisting or redundancy
elimination.
Committed: https://code.google.com/p/dart/source/detail?r=12245

[srdjan, 2012-09-12 07:06:40]

LGTM with comments & questions

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

https://chromiumcodereview.appspot.com/10911214/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_optimizer.cc:357: AbstractType::ZoneHandle());
Add:
    elements->set_result_cid(kArrayCid);

As type of elements use:
   Type::ZoneHandle(Type::DynamicType())

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

https://chromiumcodereview.appspot.com/10911214/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:79: (immutable_ == other_load->immutable_));
Also assert that result_cid is the same if offset is the same.

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

https://chromiumcodereview.appspot.com/10911214/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:2510: bool immutable = false)
I think setting immutable_ via a setter (similar to set_result_cid) is better:
no default arguments needed and better documentation at call site than passing
true/false. Otherwise, please make immutable_const.

https://chromiumcodereview.appspot.com/10911214/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language_ia32.cc (left):

https://chromiumcodereview.appspot.com/10911214/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:870: (receiver_type() ==
kImmutableArrayCid));
Why is this assert removed? Shouldn't it check that type is either kArrayCid or
kImmutableArrayCid? MAybe the assert belongs to EmitNativeCode?

https://chromiumcodereview.appspot.com/10911214/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:916: (receiver_type() == kArrayCid));
Why is this assert removed? Shouldn't we check for kArrayCid only?

[Florian Schneider, 2012-09-12 08:27:29]

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

https://chromiumcodereview.appspot.com/10911214/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_optimizer.cc:357: AbstractType::ZoneHandle());
On 2012/09/12 07:06:40, srdjan wrote:
> Add:
>     elements->set_result_cid(kArrayCid);
> 
> As type of elements use:
>    Type::ZoneHandle(Type::DynamicType())

Done.

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

https://chromiumcodereview.appspot.com/10911214/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.cc:79: (immutable_ == other_load->immutable_));
On 2012/09/12 07:06:40, srdjan wrote:
> Also assert that result_cid is the same if offset is the same.

Done.

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

https://chromiumcodereview.appspot.com/10911214/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:2510: bool immutable = false)
On 2012/09/12 07:06:40, srdjan wrote:
> I think setting immutable_ via a setter (similar to set_result_cid) is better:
> no default arguments needed and better documentation at call site than passing
> true/false. Otherwise, please make immutable_const.

Done. Making it const for now.

https://chromiumcodereview.appspot.com/10911214/diff/1/runtime/vm/intermediat...
File runtime/vm/intermediate_language_ia32.cc (left):

https://chromiumcodereview.appspot.com/10911214/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:870: (receiver_type() ==
kImmutableArrayCid));
On 2012/09/12 07:06:40, srdjan wrote:
> Why is this assert removed? Shouldn't it check that type is either kArrayCid
or
> kImmutableArrayCid? MAybe the assert belongs to EmitNativeCode?

I removed receiver_type() from LoadIndexed completely since it is not needed
anymore.

https://chromiumcodereview.appspot.com/10911214/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:916: (receiver_type() == kArrayCid));
On 2012/09/12 07:06:40, srdjan wrote:
> Why is this assert removed? Shouldn't we check for kArrayCid only?

I removed receiver_type() from StoreIndexed as well.