Unbox phis that were proven to be of type Double.

Unbox phis that were proven to be of type Double.

Eliminate Boxing/Unboxing pairs.

Allow boxing, unboxing and double binary operations to participate in CSE.

Allow double comparisons to operate on unboxed inputs.

Support XMM registers and double spill slots in deoptimization.

Save XMM registers when calling to runtime from WriteBarrier stub.

R=srdjan@google.com
BUG=

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

[srdjan, 2012-08-30 17:59:24]

LGTM with comments

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/assembler_i...
File runtime/vm/assembler_ia32.cc (right):

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/assembler_i...
runtime/vm/assembler_ia32.cc:1543: kNumberOfVolatileCpuRegisters +
(kNumberOfXmmRegisters - 1);
Why -1? Please add comment XMM0 not preserved and why not.

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

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/assembler_x...
runtime/vm/assembler_x64.cc:1697: kNumberOfVolatileCpuRegisters +
(kNumberOfXmmRegisters - 1);
ditto

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/code_genera...
File runtime/vm/code_generator.cc (right):

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/code_genera...
runtime/vm/code_generator.cc:1639: OS::Print("materialing double at %p: %g\n",
Instead of %p use PRIxPTR.
s/materialing/materializing"

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

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_allocator.cc:1447: // address as an index for the double
spill slot.  In terms of indexes
Two spaces after period.

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

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_optimizer.cc:60: Instruction* deopt_target) {
s/Convertion/Conversion/

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

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:244: virtual Representation
RequiredInputRepresentation(intptr_t i) const {
Add comment (or change parameter name) describing what is 'i'.

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:254: // to.
Please add: can return kNoDeoptId.

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:269: intptr_t deopt_id_;
Optional: do you need to be able to set it in any of subclasses? If not, you may
put in protected "get_deopt_id()" or something similar

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:776: virtual Representation
RequiredInputRepresentation(intptr_t i) const {
Asssert i is 0 or 1.

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:828: virtual Representation
RequiredInputRepresentation(intptr_t i) const {
ASSERT(i is 0, 1) ?

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1649: virtual bool
AttributesEqual(Computation* other) const { return true; }
Always true, regardless of the value? Please add comment why.

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:2251: virtual Representation
RequiredInputRepresentation(intptr_t i) const {
Again, please document i.

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

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/isolate.h#n...
runtime/vm/isolate.h:41: class DeferredDouble {
Add very brief comment what this class is for.

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/isolate.h#n...
runtime/vm/isolate.h:48: DeferredDouble* next() { return next_; }
const

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/stub_code_i...
File runtime/vm/stub_code_ia32.cc (right):

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/stub_code_i...
runtime/vm/stub_code_ia32.cc:537: __ EnterFrame(0);
Add comment somewhere that GC can occur only at call to
kDeoptimizeMaterializeDoublesRuntimeEntry)

[Vyacheslav Egorov (Google), 2012-09-21 12:51:52]

publishing old drafts

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/assembler_i...
File runtime/vm/assembler_ia32.cc (right):

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/assembler_i...
runtime/vm/assembler_ia32.cc:1543: kNumberOfVolatileCpuRegisters +
(kNumberOfXmmRegisters - 1);
On 2012/08/30 17:59:24, srdjan wrote:
> Why -1? Please add comment XMM0 not preserved and why not.

Done.

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

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/assembler_x...
runtime/vm/assembler_x64.cc:1697: kNumberOfVolatileCpuRegisters +
(kNumberOfXmmRegisters - 1);
On 2012/08/30 17:59:24, srdjan wrote:
> ditto

Done.

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/code_genera...
File runtime/vm/code_generator.cc (right):

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/code_genera...
runtime/vm/code_generator.cc:1639: OS::Print("materialing double at %p: %g\n",
On 2012/08/30 17:59:24, srdjan wrote:
> Instead of %p use PRIxPTR.
> s/materialing/materializing"

Done.

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

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_allocator.cc:1447: // address as an index for the double
spill slot.  In terms of indexes
On 2012/08/30 17:59:24, srdjan wrote:
> Two spaces after period.

Done.

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

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_optimizer.cc:60: Instruction* deopt_target) {
On 2012/08/30 17:59:24, srdjan wrote:
> s/Convertion/Conversion/

Done.

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

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:244: virtual Representation
RequiredInputRepresentation(intptr_t i) const {
On 2012/08/30 17:59:24, srdjan wrote:
> Add comment (or change parameter name) describing what is 'i'.

Done.

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:254: // to.
On 2012/08/30 17:59:24, srdjan wrote:
> Please add: can return kNoDeoptId.

Done.

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:269: intptr_t deopt_id_;
On 2012/08/30 17:59:24, srdjan wrote:
> Optional: do you need to be able to set it in any of subclasses? If not, you
may
> put in protected "get_deopt_id()" or something similar

Done.

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:776: virtual Representation
RequiredInputRepresentation(intptr_t i) const {
On 2012/08/30 17:59:24, srdjan wrote:
> Asssert i is 0 or 1.

Done.

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:828: virtual Representation
RequiredInputRepresentation(intptr_t i) const {
On 2012/08/30 17:59:24, srdjan wrote:
> ASSERT(i is 0, 1) ?

Done.

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:1649: virtual bool
AttributesEqual(Computation* other) const { return true; }
On 2012/08/30 17:59:24, srdjan wrote:
> Always true, regardless of the value? Please add comment why.

I am not sure what to add. 

AttributesEqual check only attributes of the instruction, inputs checking is
handled outside by a generic code.  

There is already a comment in the base class:

  // Compare attributes of an computation (except input operands and kind).

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/intermediat...
runtime/vm/intermediate_language.h:2251: virtual Representation
RequiredInputRepresentation(intptr_t i) const {
On 2012/08/30 17:59:24, srdjan wrote:
> Again, please document i.

Done.

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

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/isolate.h#n...
runtime/vm/isolate.h:41: class DeferredDouble {
On 2012/08/30 17:59:24, srdjan wrote:
> Add very brief comment what this class is for.

Done.

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/isolate.h#n...
runtime/vm/isolate.h:48: DeferredDouble* next() { return next_; }
On 2012/08/30 17:59:24, srdjan wrote:
> const

Done.

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/stub_code_i...
File runtime/vm/stub_code_ia32.cc (right):

https://chromiumcodereview.appspot.com/10919008/diff/1/runtime/vm/stub_code_i...
runtime/vm/stub_code_ia32.cc:537: __ EnterFrame(0);
On 2012/08/30 17:59:24, srdjan wrote:
> Add comment somewhere that GC can occur only at call to
> kDeoptimizeMaterializeDoublesRuntimeEntry)

Done.