LGTM.
http://codereview.chromium.org/2885018/diff/1/3 File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/2885018/diff/1/3#newcode9965 src/x64/codegen-x64.cc:9965: // TODO(X64): On Win64, if we ever use XMM6-XMM15, the low low 64 bits are the low low? http://codereview.chromium.org/2885018/diff/1/5 File src/x64/macro-assembler-x64.cc (right): http://codereview.chromium.org/2885018/diff/1/5#newcode559 src/x64/macro-assembler-x64.cc:559: //imul(dst, kSmiConstantRegister, Immediate(value)); Commented code. It's a bad thing. http://codereview.chromium.org/2885018/diff/1/5#newcode727 src/x64/macro-assembler-x64.cc:727: // Make mask 0x8000000000000001 and test that both bits are zero. How about movq(scratch, src) rolq(scratch, 1) testb(scratch, 3). or (the best, because it is least data-dependent?) mov(scratch, 3) ror(scratch, 1) test(scratch, src) or, weirdly: rol(src, 1) test(src, 3) ror(src, 1) http://codereview.chromium.org/2885018/diff/1/5#newcode775 src/x64/macro-assembler-x64.cc:775: cmpq(kScratchRegister, src); Why not cmpq(src, kSmiConstantRegister) return overflow Unless you are also testing for Sminess at the same time, minSmi is the only Smi that overflows when subtracting Smi 1. http://codereview.chromium.org/2885018/diff/1/5#newcode886 src/x64/macro-assembler-x64.cc:886: subq(src1, kScratchRegister); How about movq(scratch, src1) subq(scratch, src2) j(overflow) movq(dst, scratch). http://codereview.chromium.org/2885018/diff/1/9 File test/cctest/test-macro-assembler-x64.cc (right): http://codereview.chromium.org/2885018/diff/1/9#newcode103 test/cctest/test-macro-assembler-x64.cc:103: __ pop(v8::internal::kSmiConstantRegister); Why not check kSmiConstantRegister before popping the old value, since this is testing code? http://codereview.chromium.org/2885018/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
