Split ToDouble into two IL instruction to make it work with SSA.

Split ToDouble into two IL instruction to make it work with SSA.

This CL removes the bailout from SSA on smi-to-double conversions.

The smi-to-double conversion is implemented as a call and requires
its operand pushed, the double-to-double conversion has normal input
operands.
Committed: https://code.google.com/p/dart/source/detail?r=10710

[srdjan, 2012-08-14 21:21:26]

LGTM after addressing comments.

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/compiler.cc
File vm/compiler.cc (right):

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/compiler.cc#newcode408
vm/compiler.cc:408: // TODO(fschneider): Print unoptimized code along with the
optimized code.
Why did you remove it if you added a TODO? Did the previous disassemble not
work?

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/flow_graph_optimize...
File vm/flow_graph_optimizer.cc (right):

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/flow_graph_optimize...
vm/flow_graph_optimizer.cc:389: (class_ids[0] == kSmiCid)) {
No else needed, previous if terminates with a return.

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
File vm/intermediate_language.h (right):

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
vm/intermediate_language.h:1713: DoubleToDoubleComp(Value* value,
InstanceCallComp* instance_call)
Don't need instance_call, pass int deopt_id and try_index.

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
vm/intermediate_language.h:1734: class SmiToDoubleComp : public
TemplateComputation<0> {
Shouldn't this be TemplateComputation<1> ?

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
File vm/intermediate_language_ia32.cc (right):

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
vm/intermediate_language_ia32.cc:1994: locs->set_temp(0,
Location::RequiresRegister());
Remove both set_temp(0, ...), kNumTemps = 0.

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
vm/intermediate_language_ia32.cc:2040: Register value = EBX;
Allocate temp for EBX and use it here.

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
vm/intermediate_language_ia32.cc:2041: __ movl(value, Address(ESP, 0));
Why not popl(value) and add value to AddDeoptStub?

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
File vm/intermediate_language_x64.cc (right):

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
vm/intermediate_language_x64.cc:2048: Register value = RBX;
Ditto as for ia32 version.

[Florian Schneider, 2012-08-15 10:27:23]

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/compiler.cc
File vm/compiler.cc (right):

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/compiler.cc#newcode408
vm/compiler.cc:408: // TODO(fschneider): Print unoptimized code along with the
optimized code.
On 2012/08/14 21:21:26, srdjan wrote:
> Why did you remove it if you added a TODO? Did the previous disassemble not
> work?

The ia32 disassembler does not work for unoptimized code that has a patched
entry code.

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/flow_graph_optimize...
File vm/flow_graph_optimizer.cc (right):

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/flow_graph_optimize...
vm/flow_graph_optimizer.cc:389: (class_ids[0] == kSmiCid)) {
On 2012/08/14 21:21:26, srdjan wrote:
> No else needed, previous if terminates with a return.

Done.

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
File vm/intermediate_language.h (right):

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
vm/intermediate_language.h:1713: DoubleToDoubleComp(Value* value,
InstanceCallComp* instance_call)
On 2012/08/14 21:21:26, srdjan wrote:
> Don't need instance_call, pass int deopt_id and try_index.

Since the deopt_id has to agree for correctness, I'd rather not pass it. Both
are duplicated from the instance call.

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
vm/intermediate_language.h:1734: class SmiToDoubleComp : public
TemplateComputation<0> {
On 2012/08/14 21:21:26, srdjan wrote:
> Shouldn't this be TemplateComputation<1> ?

This instruction is implemented as call with pushed arguments. Therefore, zero
inputs. The flow graph optimizer does not remove the PushArgument instruction
for its input in this case.

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
vm/intermediate_language.h:1739: Value* value() const { return inputs_[0]; }
I forgot to remove this function. I'll remove it before committing.

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
File vm/intermediate_language_ia32.cc (right):

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
vm/intermediate_language_ia32.cc:1994: locs->set_temp(0,
Location::RequiresRegister());
On 2012/08/14 21:21:26, srdjan wrote:
> Remove both set_temp(0, ...), kNumTemps = 0.

ia32 needs a temp register for the CompareClassId.

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
vm/intermediate_language_ia32.cc:2040: Register value = EBX;
On 2012/08/14 21:21:26, srdjan wrote:
> Allocate temp for EBX and use it here.

Since this instruction is a call, we can freely use all registers as temps.

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
vm/intermediate_language_ia32.cc:2041: __ movl(value, Address(ESP, 0));
On 2012/08/14 21:21:26, srdjan wrote:
> Why not popl(value) and add value to AddDeoptStub?

This is because of a subtle problem with deoptimiation. Maybe I should add a
comment: We need to preserve the arguemt until after the deoptimization point
because the deopt info assumes that the value is on the stack.

This instruction should probably be redesigned in a future CL, once we have
deferred code for the allocation call and can inline new-space allocation of
doubles.

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
File vm/intermediate_language_x64.cc (right):

https://chromiumcodereview.appspot.com/10834311/diff/1/vm/intermediate_langua...
vm/intermediate_language_x64.cc:2048: Register value = RBX;
On 2012/08/14 21:21:26, srdjan wrote:
> Ditto as for ia32 version.

Same as x64.