http://codereview.chromium.org/6366010/diff/1/src/x64/assembler-x64.h File src/x64/assembler-x64.h (right):
http://codereview.chromium.org/6366010/diff/1/src/x64/assembler-x64.h#newcode396 src/x64/assembler-x64.h:396: bool depends_on_register(Register reg) const { It should probably even be AddressUsesRegister. And it's a big function. I'll move it to the .cc file. http://codereview.chromium.org/6366010/diff/1/src/x64/assembler-x64.h#newcode405 src/x64/assembler-x64.h:405: // Index code (including REX.X) of 0x04 (esp) means no index register. On 2011/01/25 07:13:28, Rico wrote:
esp -> rsp
Done. http://codereview.chromium.org/6366010/diff/1/src/x64/assembler-x64.h#newcode406 src/x64/assembler-x64.h:406: if (index_code != 0x04 && index_code == code) return true; Done, and also rbp.code() instead of 0x05 later. http://codereview.chromium.org/6366010/diff/1/src/x64/assembler-x64.h#newcode415 src/x64/assembler-x64.h:415: // Comparison uses REX.B if using the SIB byte, otherwise not. I have rewritten it so the two branches have no shared code. They now differ on when the REX.B bit is added to the base_code value (before or after testing for r/m=rbp,mod=0). http://codereview.chromium.org/6366010/diff/1/src/x64/assembler-x64.h#newcode851 src/x64/assembler-x64.h:851: void andl(Register dst, const Operand& src) { This one is already there (it's one of the fairly generic "arithmetic" operations, where we are just adding another case). http://codereview.chromium.org/6366010/diff/1/src/x64/assembler-x64.h#newcode1239 src/x64/assembler-x64.h:1239: void movdqa(const Operand& dst, XMMRegister src); On 2011/01/24 16:25:27, William Hesse wrote:
Add to disassembler.
Done. http://codereview.chromium.org/6366010/diff/1/src/x64/assembler-x64.h#newcode1282 src/x64/assembler-x64.h:1282: // Use either movsd or movlpd. On 2011/01/24 16:25:27, William Hesse wrote:
I commented these, but did not remove them. We should remove them.
Done. http://codereview.chromium.org/6366010/diff/1/src/x64/lithium-codegen-x64.cc File src/x64/lithium-codegen-x64.cc (right): http://codereview.chromium.org/6366010/diff/1/src/x64/lithium-codegen-x64.cc#newcode1157 src/x64/lithium-codegen-x64.cc:1157: __ movl(result, Immediate(Heap::kFalseValueRootIndex)); The change to the code relative to ia32 is because the ia32 code does two loads. In ia32, the loads are immediate. This was the simplest change of that code to use the root array. I have rewritten the code to use two labels and one direct root-array load per branch. http://codereview.chromium.org/6366010/diff/1/src/x64/lithium-codegen-x64.cc#newcode1285 src/x64/lithium-codegen-x64.cc:1285: Register temp = ToRegister(instr->TempAt(0)); Removed, and EmitIsObject rewritten to use only kScratchRegister as temporary. http://codereview.chromium.org/6366010/diff/1/src/x64/lithium-codegen-x64.cc#newcode1331 src/x64/lithium-codegen-x64.cc:1331: Heap::kTrueValueRootIndex)); Well spotted. http://codereview.chromium.org/6366010/diff/1/src/x64/lithium-codegen-x64.cc#newcode1467 src/x64/lithium-codegen-x64.cc:1467: Register temp = ToRegister(instr->TempAt(0)); This temp is needed in EmitClassOfTest. It uses the temp register in or through macros that clobber kScratchRegister. Remember kScratchRegister is the macro assembler's scratch register. It's not guaranteed to survive past any macro that doesn't say that it doesn't clobber it. http://codereview.chromium.org/6366010/diff/1/src/x64/lithium-codegen-x64.cc#newcode1805 src/x64/lithium-codegen-x64.cc:1805: __ cvtlsi2sd(ToDoubleRegister(output), ToOperand(input)); We assert just above that input is not a register (as does the ia32 code). http://codereview.chromium.org/6366010/diff/1/src/x64/lithium-codegen-x64.cc#newcode1848 src/x64/lithium-codegen-x64.cc:1848: __ Set(reg, 0); By chance, since it's the representation of Smi zero. I'll make it explicit, and use Move(reg, Smi::fromInt(0)); It'll emit the same code anyway (xorl(reg,reg)). http://codereview.chromium.org/6366010/diff/1/src/x64/lithium-codegen-x64.cc#newcode1901 src/x64/lithium-codegen-x64.cc:1901: __ Integer32ToSmi(input_reg, input_reg); // Retag smi. Using kScratchRegister seems like the easy way. We still have to untag it first. Done. http://codereview.chromium.org/6366010/diff/1/src/x64/macro-assembler-x64.cc File src/x64/macro-assembler-x64.cc (right): http://codereview.chromium.org/6366010/diff/1/src/x64/macro-assembler-x64.cc#newcode995 src/x64/macro-assembler-x64.cc:995: movl(dst, src); The second choice has a following operation that depends on a memory load. The first choice's movl is independent of other operations and can be hoisted to before the result is ready in src, leaving only the memory operation. So, I'm expecting marginally better parallelization. It might be pure speculation though. http://codereview.chromium.org/6366010/diff/1/src/x64/macro-assembler-x64.cc#newcode1840 src/x64/macro-assembler-x64.cc:1840: const int kShaddowSpace = 4; I believe it's the space reserved on the stack for the register-passed arguments in the Win64 calling convention. The name 'shadow space' is used by the MSDN documentation, e.g., http://msdn.microsoft.com/en-us/library/zthk2dkh.aspx The double-d is inexcusable though. http://codereview.chromium.org/6366010/diff/1/src/x64/macro-assembler-x64.cc#newcode1852 src/x64/macro-assembler-x64.cc:1852: movdqa(Operand(rbp, offset - ((i + 1) * kDoubleSize)), reg); Ack, I don't. We are only 8-byte aligned. Using movdqa is bad. Will change to movsd. http://codereview.chromium.org/6366010/diff/1/src/x64/macro-assembler-x64.cc#newcode1855 src/x64/macro-assembler-x64.cc:1855: subq(rsp, Immediate(arg_stack_space * kPointerSize)); Argh, removed. http://codereview.chromium.org/6366010/diff/1/src/x64/macro-assembler-x64.cc#newcode1900 src/x64/macro-assembler-x64.cc:1900: movdqa(reg, Operand(rbp, offset - ((i + 1) * kDoubleSize))); It seems so. It's equivalent to the ia32 code. http://codereview.chromium.org/6366010/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
