Implement SmiToInt and DoubleToInt.

Implement SmiToInt and DoubleToInt. All necessary tests are in tests/language/arithmetic_test.dart
Committed: https://code.google.com/p/dart/source/detail?r=13443

[cshapiro, 2012-10-09 00:59:23]

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
File runtime/vm/intermediate_language_x64.cc (right):

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
runtime/vm/intermediate_language_x64.cc:1994: __ cmpq(result,
Immediate(0xC000000000000000));
Why not use SH

 SHL result, 1
 JO <overflow>

The overflow bit is set to the XOR of the CF and the MSB.

If the value of result is ever needed you can

  RCR result, 1

to restore the MSB from the CF.  With an argument of 1 this is a single cycle
instruction.

[Florian Schneider, 2012-10-09 11:13:58]

LGTM with comments

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/flow_graph_opt...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/flow_graph_opt...
runtime/vm/flow_graph_optimizer.cc:927: call->ReplaceWith(int2int_instr,
current_iterator());
Since IntegerToInteger itself is a nop, I think you can avoid having an extra IL
instruction for Integer.toInt. Just do a CheckSmi/ClassCheck and then replace
all uses of the call with the input:

call->ReplaceUsesWith(call->ArgumentAt(0)->value());

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
File runtime/vm/intermediate_language_ia32.cc (right):

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
runtime/vm/intermediate_language_ia32.cc:2020: __ movl(value_obj, Address(ESP,
0));
Alternatively, you could make the instruction take 1 input operand in a register
and push the argument explicitly before the call and drop it after the call.

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
runtime/vm/intermediate_language_ia32.cc:2022: __ cvttsd2si(result,
value_double);
Please add this instruction to the disassembler. I noticed it is still missing.

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
File runtime/vm/intermediate_language_x64.cc (right):

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
runtime/vm/intermediate_language_x64.cc:1990: __ cvttsd2siq(result,
value_double);
Please make sure that the tests cover the corner cases like NaN and +/-Infinity.

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
runtime/vm/intermediate_language_x64.cc:2010: Array::Handle(),  // No argument
names.,
Remove extra , at the end.

[srdjan, 2012-10-09 21:23:04]

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/flow_graph_opt...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/flow_graph_opt...
runtime/vm/flow_graph_optimizer.cc:927: call->ReplaceWith(int2int_instr,
current_iterator());
On 2012/10/09 11:13:58, Florian Schneider wrote:
> Since IntegerToInteger itself is a nop, I think you can avoid having an extra
IL
> instruction for Integer.toInt. Just do a CheckSmi/ClassCheck and then replace
> all uses of the call with the input:
> 
> call->ReplaceUsesWith(call->ArgumentAt(0)->value());
> 

Done.

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
File runtime/vm/intermediate_language_ia32.cc (right):

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
runtime/vm/intermediate_language_ia32.cc:2020: __ movl(value_obj, Address(ESP,
0));
On 2012/10/09 11:13:58, Florian Schneider wrote:
> Alternatively, you could make the instruction take 1 input operand in a
register
> and push the argument explicitly before the call and drop it after the call.

Done.

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
runtime/vm/intermediate_language_ia32.cc:2022: __ cvttsd2si(result,
value_double);
On 2012/10/09 11:13:58, Florian Schneider wrote:
> Please add this instruction to the disassembler. I noticed it is still
missing.

Done.

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
File runtime/vm/intermediate_language_x64.cc (right):

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
runtime/vm/intermediate_language_x64.cc:1990: __ cvttsd2siq(result,
value_double);
On 2012/10/09 11:13:58, Florian Schneider wrote:
> Please make sure that the tests cover the corner cases like NaN and
+/-Infinity.

Yes, this is covered in arithmetic_test.dart (throw exception).

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
runtime/vm/intermediate_language_x64.cc:1994: __ cmpq(result,
Immediate(0xC000000000000000));
On 2012/10/09 00:59:24, cshapiro wrote:
> Why not use SH
> 
>  SHL result, 1
>  JO <overflow>
> 
> The overflow bit is set to the XOR of the CF and the MSB.
> 
> If the value of result is ever needed you can
> 
>   RCR result, 1
> 
> to restore the MSB from the CF.  With an argument of 1 this is a single cycle
> instruction.

Done, using a temporary register to preserve result.

https://codereview.chromium.org/11098009/diff/10001/runtime/vm/intermediate_l...
runtime/vm/intermediate_language_x64.cc:2010: Array::Handle(),  // No argument
names.,
On 2012/10/09 11:13:58, Florian Schneider wrote:
> Remove extra , at the end.

Done.