http://codereview.chromium.org/1860001/diff/1/5 File src/x64/codegen-x64.cc (right):
http://codereview.chromium.org/1860001/diff/1/5#newcode7404 src/x64/codegen-x64.cc:7404: // Test that eax is a number. On 2010/05/03 09:14:46, Mads Ager wrote:
eax -> rax
Move to after the label declarations?
Done. http://codereview.chromium.org/1860001/diff/1/5#newcode7413 src/x64/codegen-x64.cc:7413: ASSERT_EQ(1, kSmiTagSize); Removed. http://codereview.chromium.org/1860001/diff/1/5#newcode7427 src/x64/codegen-x64.cc:7427: __ cmpq(rbx, FieldOperand(rax, HeapObject::kMapOffset)); We can, but in this case I don't want to. Using the root array costs a load and is somewhat shorter, but I'm afraid the load can't be hoisted very much since we are just after a conditional jump target, so we will be introducing extra latency. http://codereview.chromium.org/1860001/diff/1/5#newcode7443 src/x64/codegen-x64.cc:7443: // or h = (h0 ^ (h0 >> 8) ^ (h0 >> 16) ^ (h0 >> 24)) & cacheSize - 1 On 2010/05/03 09:14:46, Mads Ager wrote:
add parenthesis (cacheSize - 1)?
Done. http://codereview.chromium.org/1860001/diff/1/5#newcode7469 src/x64/codegen-x64.cc:7469: { // NOLINT - doesn't like a single brace on a line. It probably will, but I'd hate it instead. I'd rather remove the braces entirely. http://codereview.chromium.org/1860001/diff/1/5#newcode7485 src/x64/codegen-x64.cc:7485: __ lea(rcx, Operand(rax, rcx, times_8, 0));\ Whoops. http://codereview.chromium.org/1860001/diff/1/5#newcode7497 src/x64/codegen-x64.cc:7497: // We are short on registers, so use no_reg as scratch. Removed. http://codereview.chromium.org/1860001/diff/1/5#newcode7526 src/x64/codegen-x64.cc:7526: // Only free register is edi. fixed. http://codereview.chromium.org/1860001/diff/1/5#newcode7540 src/x64/codegen-x64.cc:7540: __ andl(rdi, Immediate(0x7ff)); Fixed. http://codereview.chromium.org/1860001/diff/1/5#newcode7542 src/x64/codegen-x64.cc:7542: (63 + HeapNumber::kExponentBias); On 2010/05/03 09:14:46, Mads Ager wrote:
This will fit on line above?
Done. http://codereview.chromium.org/1860001/diff/1/5#newcode7552 src/x64/codegen-x64.cc:7552: __ movq(rdi, static_cast<uint64_t>(0x7ff8)<<48, RelocInfo::NONE); On 2010/05/03 09:14:46, Mads Ager wrote:
We should use a named constant for the nan representation.
Done. http://codereview.chromium.org/1860001/diff/1/5#newcode7584 src/x64/codegen-x64.cc:7584: __ testl(rax, Immediate(0x400 /* C2 */)); C2 moved to comment. http://codereview.chromium.org/1860001/diff/1/5#newcode7594 src/x64/codegen-x64.cc:7594: __ movq(rax, rdi); // Restore eax (allocated HeapNumber pointer). eax->rax. http://codereview.chromium.org/1860001/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
