Add support for XMM registers in SSA code generation pipeline.

Add support for XMM registers in SSA code generation pipeline.

Split BinaryDoubleOp into several instructions that manipulate unboxed doubles.

R=fschneider@google.com
BUG=

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

[Florian Schneider, 2012-08-24 11:09:27]

Could you upload a rebased version so that I can look at flow_graph_optimizer
again with the new use-chains?

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

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_allocator.cc:444: } else if (loc.IsXmmRegister()) {
A little shorter:

} else {
  ASSERT(loc.IsXmmRegister());
  BlockRegisterLocation(loc, from, to, blocked_xmm_registers_, xmm_regs_);
}

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_allocator.cc:568: // All incomming parameters are tagged.
s/incomming/incoming/

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_allocator.cc:766: } else {
Maybe add 

ASSERT(loc.policy() == Location::kRequiresRegister)

here.

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_allocator.cc:1447:
Location::DoubleStackSlot(cpu_spill_slot_count_ + idx * 2));
Why always times 2? Use sizeof instead. double spill slot is 64bit. Normal spill
slot is 32 or 64.

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_allocator.cc:2135: (deferred_xmm_spills +
spill_slots_.length()) * 2 +
Why times 2?

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

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_optimizer.cc:257: bind->set_env(env->Clone());
Use Environment::Copy?

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_optimizer.cc:343: InsertBefore(instr, new
UnboxDoubleComp(left, comp), instr->env());
Since left and right are used multiple times now, you need to do CopyValue for
each new use.

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

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1688: intptr_t deopt_id() const { return
original_->deopt_id(); }
Either add accessors to the other deoptimizing instructions like UnboxDoubleComp
as well, or always access deopt_id, try_index via the original()->deopt_id() in
the code generator (which is currently the more common style)

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1692: InstanceCallComp* original_;
Maybe rename to instance_call_ for consistency?

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1733: virtual bool CanDeoptimize() const {
return true; }
More precisely:

return (value()->ResultCid() != kDoubleCid);

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1781: InstanceCallComp* instance_call_;
No need for instance_call_ here. This instruction does not deoptimize and does
not need deopt_id, etc. from the original instance call.

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

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:1810: if (left()->ResultCid() ==
kDoubleCid || right()->ResultCid() == kDoubleCid) {
I'd rather eliminate the check by overriding Computation::TryReplace.

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:1865:
compiler->RestoreLiveRegisters(locs);
This only works if the output register is not live here.

Maybe ASSERT(!locs->live_registers->Contain(locs->out().reg()));

Or change the order of restore and assert that EAX is not in the live registers.

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:1867: // TODO(vegorov): wrong order of
save restore if EAX is live across this
How can that happen?

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/locations.h
File runtime/vm/locations.h (right):

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/locations.h...
runtime/vm/locations.h:180: 
-1 line.

[Vyacheslav Egorov (Google), 2012-08-24 13:23:01]

Addressed comments. Rebased. 

Please take another look.

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

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_allocator.cc:444: } else if (loc.IsXmmRegister()) {
On 2012/08/24 11:09:28, Florian Schneider wrote:
> A little shorter:
> 
> } else {
>   ASSERT(loc.IsXmmRegister());
>   BlockRegisterLocation(loc, from, to, blocked_xmm_registers_, xmm_regs_);
> }

Done.

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

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_optimizer.cc:257: bind->set_env(env->Clone());
On 2012/08/24 11:09:28, Florian Schneider wrote:
> Use Environment::Copy?

Done.

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_optimizer.cc:343: InsertBefore(instr, new
UnboxDoubleComp(left, comp), instr->env());
On 2012/08/24 11:09:28, Florian Schneider wrote:
> Since left and right are used multiple times now, you need to do CopyValue for
> each new use.

Done.

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

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1688: intptr_t deopt_id() const { return
original_->deopt_id(); }
On 2012/08/24 11:09:28, Florian Schneider wrote:
> Either add accessors to the other deoptimizing instructions like
UnboxDoubleComp
> as well, or always access deopt_id, try_index via the original()->deopt_id()
in
> the code generator (which is currently the more common style)

Done.

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1688: intptr_t deopt_id() const { return
original_->deopt_id(); }
On 2012/08/24 11:09:28, Florian Schneider wrote:
> Either add accessors to the other deoptimizing instructions like
UnboxDoubleComp
> as well, or always access deopt_id, try_index via the original()->deopt_id()
in
> the code generator (which is currently the more common style)

Done.

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1692: InstanceCallComp* original_;
On 2012/08/24 11:09:28, Florian Schneider wrote:
> Maybe rename to instance_call_ for consistency?

Done.

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1733: virtual bool CanDeoptimize() const {
return true; }
On 2012/08/24 11:09:28, Florian Schneider wrote:
> More precisely:
> 
> return (value()->ResultCid() != kDoubleCid);

Done.

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1781: InstanceCallComp* instance_call_;
On 2012/08/24 11:09:28, Florian Schneider wrote:
> No need for instance_call_ here. This instruction does not deoptimize and does
> not need deopt_id, etc. from the original instance call.

Done.

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

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:1810: if (left()->ResultCid() ==
kDoubleCid || right()->ResultCid() == kDoubleCid) {
On 2012/08/24 11:09:28, Florian Schneider wrote:
> I'd rather eliminate the check by overriding Computation::TryReplace.

Done.

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:1865:
compiler->RestoreLiveRegisters(locs);
On 2012/08/24 11:09:28, Florian Schneider wrote:
> This only works if the output register is not live here.
> 
> Maybe ASSERT(!locs->live_registers->Contain(locs->out().reg()));
> 
> Or change the order of restore and assert that EAX is not in the live
registers.

locs->live_registers()->Remove(locs->out()); <- this line takes care of correct
restoration, see above

https://chromiumcodereview.appspot.com/10875030/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_ia32.cc:1867: // TODO(vegorov): wrong order of
save restore if EAX is live across this
On 2012/08/24 11:09:28, Florian Schneider wrote:
> How can that happen?

This comment is left over from before it was fixed.

[Florian Schneider, 2012-08-24 13:40:55]

lgtm

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

https://chromiumcodereview.appspot.com/10875030/diff/5002/runtime/vm/intermed...
runtime/vm/intermediate_language.h:1699: Value* right() const { return
inputs_[0]; }
right should be inputs_[1]. If the left/right-accessors are never used, consider
deleting them.