LGTM
http://codereview.chromium.org/2805046/diff/1/3 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/2805046/diff/1/3#newcode11905 src/ia32/codegen-ia32.cc:11905: __ bind(&call_builtin); Seems odd to put more code at the call_builtin label. The label should probably be renamed to something else. http://codereview.chromium.org/2805046/diff/1/3#newcode11911 src/ia32/codegen-ia32.cc:11911: // At most one is a smi, so we can test for smi by adding the two. More explanation of why this works. Perhaps just "If one is a smi, the low bit is 1, if both are non-smi, it is zero." http://codereview.chromium.org/2805046/diff/1/3#newcode11916 src/ia32/codegen-ia32.cc:11916: __ j(not_zero, ¬_shown_unequal); Maybe change label to "not_both_detectable_js_objects". http://codereview.chromium.org/2805046/diff/1/3#newcode11926 src/ia32/codegen-ia32.cc:11926: __ j(not_zero, ¬_shown_unequal); Could you load both bitfields into ecx and ebx and or them and then only do one test? http://codereview.chromium.org/2805046/diff/1/4 File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/2805046/diff/1/4#newcode9132 src/x64/codegen-x64.cc:9132: Condition a_smi = masm->CheckEitherSmi(rax, rdx); The CheckEitherSmi macro seems less efficient than what you have in the 32-bit version. Maybe add a macro for CheckEitherSmiGivenNotBothSmi (with, hopefully, a better name). http://codereview.chromium.org/2805046/diff/1/4#newcode9146 src/x64/codegen-x64.cc:9146: __ bind(¬_shown_unequal); Same comments as for ia32 applies here. http://codereview.chromium.org/2805046/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
