Implement inlined version of binary arithmetic operations for doubles.

Implement inlined version of binary arithmetic operations for doubles.

No attempt to reuse temporary boxes or locally propagate types is done. 

BUG=
TEST=

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

[srdjan, 2012-06-08 05:18:22]

LGTM with comments

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/assembler_x...
File runtime/vm/assembler_x64.cc (right):

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/assembler_x...
runtime/vm/assembler_x64.cc:560: EmitOperand(dst, operand);
Please add a test in assembler_x64_test (see test for cvtsi2sd in
assembler_ia32_test.cc)

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

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_optimizer.cc:123: op_kind != Token::kDIV) {
TODO(vegorov)  ;-)?

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

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1331: const OperandsType operands_type_;
This works for now, but in Dart it is quite frequent that we have to support a
mix of double and int types (result is always double). Sometimes it is a mix
Smi/Smi and Smi/Double.. maybe that can be handled by additional OperandsTypes?

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

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_x64.cc:940: static void
LoadDoubleOrSmi(FlowGraphCompiler* compiler,
LoadDoubleOrSmiToXMM?

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_x64.cc:940: static void
LoadDoubleOrSmi(FlowGraphCompiler* compiler,
WHy not move it into FlowGraphCompiler and eliminate the first argument?

[Mads Ager (google), 2012-06-08 09:15:53]

DBC :-)

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

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_x64.cc:884: return;
Why 'return' here and 'break' below?

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_x64.cc:936: return;
Why the explicit return?

[Vyacheslav Egorov (Google), 2012-06-08 11:11:13]

Thanks for the review, landing.

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/assembler_x...
File runtime/vm/assembler_x64.cc (right):

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/assembler_x...
runtime/vm/assembler_x64.cc:560: EmitOperand(dst, operand);
On 2012/06/08 05:18:22, srdjan wrote:
> Please add a test in assembler_x64_test (see test for cvtsi2sd in
> assembler_ia32_test.cc)

done

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

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_optimizer.cc:123: op_kind != Token::kDIV) {
On 2012/06/08 05:18:22, srdjan wrote:
> TODO(vegorov)  ;-)?

Done.

I think they are not supported even by ia32 opt compiler :-)

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

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1331: const OperandsType operands_type_;
On 2012/06/08 05:18:22, srdjan wrote:
> This works for now, but in Dart it is quite frequent that we have to support a
> mix of double and int types (result is always double). Sometimes it is a mix
> Smi/Smi and Smi/Double.. maybe that can be handled by additional
OperandsTypes?

We will have to decide how we want to implement them. For example V8 decision
was that binary arithmetic operation always has a single type and inputs are
coerced explicitly in the IR:

d1 = <...> ; real double
d2 = Change i to d i4 ; int operand was coerced to double
d3 = mul-d d1, d2

The benefit of handling conversions outside of the op is that you can optimize
away redundancies and select single representation for loop phies which is very
important for performance of tight loops.

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

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_x64.cc:884: return;
On 2012/06/08 09:15:53, Mads Ager wrote:
> Why 'return' here and 'break' below?

Done.

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_x64.cc:936: return;
On 2012/06/08 09:15:53, Mads Ager wrote:
> Why the explicit return?

Done.

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_x64.cc:940: static void
LoadDoubleOrSmi(FlowGraphCompiler* compiler,
On 2012/06/08 05:18:22, srdjan wrote:
> LoadDoubleOrSmiToXMM?

Done.

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language_x64.cc:940: static void
LoadDoubleOrSmi(FlowGraphCompiler* compiler,
On 2012/06/08 05:18:22, srdjan wrote:
> WHy not move it into FlowGraphCompiler and eliminate the first argument?

Done.

[srdjan, 2012-06-08 16:05:55]

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

https://chromiumcodereview.appspot.com/10533053/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1331: const OperandsType operands_type_;
On 2012/06/08 11:11:13, Vyacheslav Egorov (Google) wrote:
> On 2012/06/08 05:18:22, srdjan wrote:
> > This works for now, but in Dart it is quite frequent that we have to support
a
> > mix of double and int types (result is always double). Sometimes it is a mix
> > Smi/Smi and Smi/Double.. maybe that can be handled by additional
> OperandsTypes?
> 
> We will have to decide how we want to implement them. For example V8 decision
> was that binary arithmetic operation always has a single type and inputs are
> coerced explicitly in the IR:
> 
> d1 = <...> ; real double
> d2 = Change i to d i4 ; int operand was coerced to double
> d3 = mul-d d1, d2
> 
> The benefit of handling conversions outside of the op is that you can optimize
> away redundancies and select single representation for loop phies which is
very
> important for performance of tight loops.

Sounds good.