Add support for deoptimization on x64. ...

Add support for deoptimization on x64. 

I did not take out the code relating to osr from the generate method
since this makes it easier to compare to ia32 (we will abort anyway when we hit the osr code so there should be no issues with having this in)
 


Committed: http://code.google.com/p/v8/source/detail?r=6449

[Lasse Reichstein, 2011-01-24 13:24:21]

LGTM, as far as I understand it :)

http://codereview.chromium.org/6390001/diff/1/src/x64/assembler-x64.h
File src/x64/assembler-x64.h (right):

http://codereview.chromium.org/6390001/diff/1/src/x64/assembler-x64.h#newcode593
src/x64/assembler-x64.h:593: void push_imm32(int32_t imm32);
Add a comment stating why it's necessary to have this function, and not just use
push(Immediate(imm32)).
(It's for "fixed code size guarantee" I assume).

http://codereview.chromium.org/6390001/diff/1/src/x64/assembler-x64.h#newcode...
src/x64/assembler-x64.h:1126: void j(Condition cc, byte* entry, RelocInfo::Mode
rmode, Hint hint= no_hint);
We don't use hints in the X64 assembler.

http://codereview.chromium.org/6390001/diff/1/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

http://codereview.chromium.org/6390001/diff/1/src/x64/builtins-x64.cc#newcode576
src/x64/builtins-x64.cc:576: __ SmiToInteger32(rcx, rcx);
You can use
 SmiToInteger32(rcx, Operand(rsp, 1 * kPointerSize);
It actually reads the 32-bit value directly from the upper bits of the stored
smi.

http://codereview.chromium.org/6390001/diff/1/src/x64/builtins-x64.cc#newcode579
src/x64/builtins-x64.cc:579: NearLabel not_no_registers, not_tos_eax;
eax->rax

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc
File src/x64/deoptimizer-x64.cc (right):

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:373: // Preserve deoptimizer object in register rax
and get the input
Indentation.

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:388: int src_offset = i * kDoubleSize +
kNumberOfRegisters * kPointerSize;
If the values are at the end of the stack, you could pop them directly to a
memory location.

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:471: ASSERT(i > 0);
Why not just 
 if (r.is(rsp)) {
   ASSERT(i > 0);
   r = Register::toRegister(i - 1);
 }
instead of unrolling the loop once.

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:484:
reinterpret_cast<uint64_t>(Smi::FromInt(kSmiConstantRegisterValue)),
Indentation.

[Rico, 2011-01-24 13:40:14]

http://codereview.chromium.org/6390001/diff/1/src/x64/assembler-x64.h
File src/x64/assembler-x64.h (right):

http://codereview.chromium.org/6390001/diff/1/src/x64/assembler-x64.h#newcode593
src/x64/assembler-x64.h:593: void push_imm32(int32_t imm32);
On 2011/01/24 13:24:21, Lasse Reichstein wrote:
> Add a comment stating why it's necessary to have this function, and not just
use
> push(Immediate(imm32)).
> (It's for "fixed code size guarantee" I assume).

Done.

http://codereview.chromium.org/6390001/diff/1/src/x64/assembler-x64.h#newcode...
src/x64/assembler-x64.h:1126: void j(Condition cc, byte* entry, RelocInfo::Mode
rmode, Hint hint= no_hint);
On 2011/01/24 13:24:21, Lasse Reichstein wrote:
> We don't use hints in the X64 assembler.

Done.

http://codereview.chromium.org/6390001/diff/1/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

http://codereview.chromium.org/6390001/diff/1/src/x64/builtins-x64.cc#newcode576
src/x64/builtins-x64.cc:576: __ SmiToInteger32(rcx, rcx);
On 2011/01/24 13:24:21, Lasse Reichstein wrote:
> You can use
>  SmiToInteger32(rcx, Operand(rsp, 1 * kPointerSize);
> It actually reads the 32-bit value directly from the upper bits of the stored
> smi.

Done.

http://codereview.chromium.org/6390001/diff/1/src/x64/builtins-x64.cc#newcode579
src/x64/builtins-x64.cc:579: NearLabel not_no_registers, not_tos_eax;
On 2011/01/24 13:24:21, Lasse Reichstein wrote:
> eax->rax

Done.

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc
File src/x64/deoptimizer-x64.cc (right):

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:373: // Preserve deoptimizer object in register rax
and get the input
On 2011/01/24 13:24:21, Lasse Reichstein wrote:
> Indentation.

Done.

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:388: int src_offset = i * kDoubleSize +
kNumberOfRegisters * kPointerSize;
On 2011/01/24 13:24:21, Lasse Reichstein wrote:
> If the values are at the end of the stack, you could pop them directly to a
> memory location.

They are not at the end, the general purpose registers are on top of these. In
addition, this makes the code very similar to the ia32 version.

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:471: ASSERT(i > 0);
On 2011/01/24 13:24:21, Lasse Reichstein wrote:
> Why not just 
>  if (r.is(rsp)) {
>    ASSERT(i > 0);
>    r = Register::toRegister(i - 1);
>  }
> instead of unrolling the loop once.

Done.

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:484:
reinterpret_cast<uint64_t>(Smi::FromInt(kSmiConstantRegisterValue)),
On 2011/01/24 13:24:21, Lasse Reichstein wrote:
> Indentation.

Done.

[Kevin Millikin (Chromium), 2011-01-24 13:52:20]

Small comments, LGTM.

http://codereview.chromium.org/6390001/diff/1/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

http://codereview.chromium.org/6390001/diff/1/src/x64/builtins-x64.cc#newcode567
src/x64/builtins-x64.cc:567: // Pass the function and deoptimization type to the
runtime system.
This comment isn't correct.  We grab the function from the stack.

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc
File src/x64/deoptimizer-x64.cc (right):

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:321: // on the stack on windows and in r8 on windows.
The remaining arguments are
Comment mentions "on windows" and "on windows".

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:380: __ movq(rcx, Operand(rsp, (kNumberOfRegisters -
1 - i) * kPointerSize));
I wonder why we don't count i down from kNumberOfRegisters to zero and

__ pop(Operand(rbx, offset))

for all of them?

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:388: int src_offset = i * kDoubleSize +
kNumberOfRegisters * kPointerSize;
Agree with lrn.

[Rico, 2011-01-25 07:48:14]

http://codereview.chromium.org/6390001/diff/1/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

http://codereview.chromium.org/6390001/diff/1/src/x64/builtins-x64.cc#newcode567
src/x64/builtins-x64.cc:567: // Pass the function and deoptimization type to the
runtime system.
On 2011/01/24 13:52:21, Kevin Millikin wrote:
> This comment isn't correct.  We grab the function from the stack.

Done.

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc
File src/x64/deoptimizer-x64.cc (right):

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:321: // on the stack on windows and in r8 on windows.
The remaining arguments are
On 2011/01/24 13:52:21, Kevin Millikin wrote:
> Comment mentions "on windows" and "on windows".

Done.

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:380: __ movq(rcx, Operand(rsp, (kNumberOfRegisters -
1 - i) * kPointerSize));
On 2011/01/24 13:52:21, Kevin Millikin wrote:
> I wonder why we don't count i down from kNumberOfRegisters to zero and
> 
> __ pop(Operand(rbx, offset))
> 
> for all of them?

Done.

http://codereview.chromium.org/6390001/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:388: int src_offset = i * kDoubleSize +
kNumberOfRegisters * kPointerSize;
On 2011/01/24 13:52:21, Kevin Millikin wrote:
> Agree with lrn.

Done.