LGTM, with changes.
http://codereview.chromium.org/2078022/diff/1/6 File src/x64/full-codegen-x64.cc (right): http://codereview.chromium.org/2078022/diff/1/6#newcode232 src/x64/full-codegen-x64.cc:232: Is there a reason not to insert this function in the same position it appears in in the ia32 implementation, and in the order it appears in in the header file? http://codereview.chromium.org/2078022/diff/1/6#newcode2028 src/x64/full-codegen-x64.cc:2028: __ CmpInstanceType(rbx, FIRST_JS_OBJECT_TYPE); These should use above and below, unless you comment that you really want to interpret non-string types as negative numbers. The equivalent ia32 code should use CmpInstanceType, as well, or else this code should be changed to match the ia32 code. And the ia32 code should use below and above, not less and greater. http://codereview.chromium.org/2078022/diff/1/6#newcode2216 src/x64/full-codegen-x64.cc:2216: __ j(less, &null); Use unsigned comparison, not signed. Also on ia32. http://codereview.chromium.org/2078022/diff/1/7 File src/x64/macro-assembler-x64.cc (right): http://codereview.chromium.org/2078022/diff/1/7#newcode924 src/x64/macro-assembler-x64.cc:924: Move(kScratchRegister, constant); Assert that dst is not kScratchRegister? http://codereview.chromium.org/2078022/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
