Add the instructions to the disassembler.
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 { Could you call it address_uses_register() ? 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. instead of "if using the SIB byte", say "if base_code came from the SIB byte". Maybe it is simpler if you have only base_code = buf_[0] & 0x7, test against 0x05, then test against 0x04, and finally inside SIB byte test the 4-bit bit code against 0x05 again. Then there is no need for the base_bits variable. 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) { Add to disassembler. 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); Add to disassembler. http://codereview.chromium.org/6366010/diff/1/src/x64/assembler-x64.h#newcode1282 src/x64/assembler-x64.h:1282: // Use either movsd or movlpd. I commented these, but did not remove them. We should remove them. 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)); Why not move True and False directly to the result? That is just Operand(kRootRegister, some offset), so I don't see why two loads is better than one. Are you thinking that you are getting an extra memory load in one of the two cases? Then move the branch before the load, and add another branch to done. Do you know this is faster than simpler ways? How do you know that if a load from memory is is immediately killed by another load, that it is actually carried out? Maybe it isn't. http://codereview.chromium.org/6366010/diff/1/src/x64/lithium-codegen-x64.cc#newcode1331 src/x64/lithium-codegen-x64.cc:1331: Heap::kTrueValueRootIndex)); I think you need to multiply the kTrueValueRootIndex by kPointerSize. 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)); Operand cannot represent a register, so you need to write two cases: a (memory) Operand, and a Register. http://codereview.chromium.org/6366010/diff/1/src/x64/lithium-codegen-x64.cc#newcode1848 src/x64/lithium-codegen-x64.cc:1848: __ Set(reg, 0); How can 0 be a valid pointer? It isn't tagged. 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. Can't we have some way to make this optional, or use kScratchRegister, so that we don't do unneccesary work (retagging the Smi). 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); Why is the second choice worst than the first. Why not just do the second choice all the time? 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; Shadow has one d. What is shadow space? 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); How do we know that we are 16-byte aligned here? I think we are only 8-byte aligned. The frame is aligned below. 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)); This code is duplicated. It seems to run twice- a bad idea. 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))); Is this the right rbp here? http://codereview.chromium.org/6366010/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
