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#newcode1126
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#newcode373
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#newcode388
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#newcode471
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#newcode484
src/x64/deoptimizer-x64.cc:484:
reinterpret_cast<uint64_t>(Smi::FromInt(kSmiConstantRegisterValue)),
Indentation.

http://codereview.chromium.org/6390001/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to