LGTM
http://codereview.chromium.org/2130003/diff/1/2 File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/2130003/diff/1/2#newcode3452 src/x64/codegen-x64.cc:3452: if (FLAG_debug_code) __ AbortIfNotSmi(new_value.reg()); __ is a macro. It may expand to more than one statement. You should wrap the then branch in brackets. http://codereview.chromium.org/2130003/diff/1/2#newcode5910 src/x64/codegen-x64.cc:5910: __ AbortIfNotSmi(left_side.reg()); Do we ever use Abort* macros not guarded by FLAG_debug_code? If not, maybe we should put the flag test into the macro, or perhaps make an AssertSmi macro for it. http://codereview.chromium.org/2130003/diff/1/3 File src/x64/macro-assembler-x64.cc (right): http://codereview.chromium.org/2130003/diff/1/3#newcode1739 src/x64/macro-assembler-x64.cc:1739: Assert(equal, "Operand not a number"); What does "operand" signify here? Operand of what? How about "Unexpected non-number". http://codereview.chromium.org/2130003/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
