LGTM
http://codereview.chromium.org/6366028/diff/6010/src/x64/code-stubs-x64.cc File src/x64/code-stubs-x64.cc (right): http://codereview.chromium.org/6366028/diff/6010/src/x64/code-stubs-x64.cc#newcode1151 src/x64/code-stubs-x64.cc:1151: __ SmiMod(rax, left, right, &restore_MOD_registers); Do you need the label "restore_MOD_registers"? It is bound the same place as "use_fp_on_smis" where there is a check for both DIV and MOD. http://codereview.chromium.org/6366028/diff/6010/src/x64/code-stubs-x64.cc#newcode1205 src/x64/code-stubs-x64.cc:1205: if (generate_inline_heapnumber_results) { Can't you take mode_ into account here and reuse an overwritable heap number if any? http://codereview.chromium.org/6366028/diff/6010/src/x64/code-stubs-x64.cc#newcode1226 src/x64/code-stubs-x64.cc:1226: if (op_ == Token::BIT_OR) { rax -> right? http://codereview.chromium.org/6366028/diff/6010/src/x64/code-stubs-x64.cc#newcode1300 src/x64/code-stubs-x64.cc:1300: // Not using AllocateHeapNumber macro in order to reuse How about creating an AllocateHeapNumber in macro assembler which takes an additional register with the heap number map? http://codereview.chromium.org/6366028/diff/6010/src/x64/code-stubs-x64.cc#newcode1347 src/x64/code-stubs-x64.cc:1347: Condition is_smi; Please remove this debugging code. http://codereview.chromium.org/6366028/diff/6010/src/x64/code-stubs-x64.cc#newcode1350 src/x64/code-stubs-x64.cc:1350: is_smi = masm->CheckSmi(lhs); The code pattern for checking for a string is repeated three times. How about JumpIfNotString in macro assembler? http://codereview.chromium.org/6366028/diff/6010/src/x64/code-stubs-x64.cc#newcode1451 src/x64/code-stubs-x64.cc:1451: __ bind(&call_runtime); Should there be a separate exit for failed heap allocation in the ALLOW_HEAPNUMBER_RESULTS case which does not perform a type transition, but just calls runtime? http://codereview.chromium.org/6366028/diff/6010/src/x64/code-stubs-x64.cc#newcode1465 src/x64/code-stubs-x64.cc:1465: Label call_runtime, type_transition; Maybe rename the labels call_runtime -> gc_required type_transition -> not_number to make it more clear why the exits are called. http://codereview.chromium.org/6366028/diff/6010/src/x64/full-codegen-x64.cc File src/x64/full-codegen-x64.cc (right): http://codereview.chromium.org/6366028/diff/6010/src/x64/full-codegen-x64.cc#newcode3209 src/x64/full-codegen-x64.cc:3209: __ j(is_smi, &done); Remove empty line? http://codereview.chromium.org/6366028/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
