LGTM
http://codereview.chromium.org/2885002/diff/19001/20001 File src/arm/assembler-arm.cc (right): http://codereview.chromium.org/2885002/diff/19001/20001#newcode2115 src/arm/assembler-arm.cc:2115: 1 blank line too many http://codereview.chromium.org/2885002/diff/26001/24004 File src/arm/codegen-arm.cc (right): http://codereview.chromium.org/2885002/diff/26001/24004#newcode4294 src/arm/codegen-arm.cc:4294: Label not_minus_half, allocate_return, runtime; If you use JumpTarget here instead of Label then you shouldn't have to handle the virtual frame manually below. http://codereview.chromium.org/2885002/diff/26001/24004#newcode4300 src/arm/codegen-arm.cc:4300: frame_->SpillAllButCopyTOSToR1R0(); I think you could just use PopToRegister here and not force a particular ordering on the two arguments. That means you have to push them again where you call the runtime, but since they remain untouched up to that point that shouldn't be so hard. http://codereview.chromium.org/2885002/diff/26001/24004#newcode4311 src/arm/codegen-arm.cc:4311: __ LoadRoot(r6, Heap::kHeapNumberMapRootIndex); Here you should use heap_number_map instead of r6. http://codereview.chromium.org/2885002/diff/26001/24004#newcode4322 src/arm/codegen-arm.cc:4322: __ b(ne, ¬_minus_half); Might as well go straight to runtime here. http://codereview.chromium.org/2885002/diff/26001/24004#newcode4334 src/arm/codegen-arm.cc:4334: __ mov(scratch2, Operand(0x3ff00000)); for a tiny microoptimization reverse the order of these two 'mov's. http://codereview.chromium.org/2885002/diff/26001/24004#newcode4347 src/arm/codegen-arm.cc:4347: __ b(ne, &runtime); You can remove this test if you went straight to runtime above. http://codereview.chromium.org/2885002/diff/26001/24004#newcode4395 src/arm/codegen-arm.cc:4395: frame()->Dup(); I think you can just push it at the point where you call the runtime. http://codereview.chromium.org/2885002/diff/26001/24006 File src/arm/macro-assembler-arm.cc (right): http://codereview.chromium.org/2885002/diff/26001/24006#newcode1400 src/arm/macro-assembler-arm.cc:1400: cmp(scratch1, Operand(HeapNumber::kExponentMask)); You can do Sbfx and then compare with -1 here. That avoids two loads to ip. http://codereview.chromium.org/2885002/diff/26001/24007 File src/arm/macro-assembler-arm.h (right): http://codereview.chromium.org/2885002/diff/26001/24007#newcode77 src/arm/macro-assembler-arm.h:77: AVOID_NANS_AND_INFINITIES = 1 << 2 This should probably be 1 << 1. http://codereview.chromium.org/2885002/diff/26001/24007#newcode498 src/arm/macro-assembler-arm.h:498: Register scratch2, I think it's rather confusing that the order of the scratch and heap_number_map registers is different here than it is in AllocateHeapNumberWithValue. http://codereview.chromium.org/2885002/diff/26001/24008 File src/arm/simulator-arm.cc (right): http://codereview.chromium.org/2885002/diff/26001/24008#newcode2291 src/arm/simulator-arm.cc:2291: double dd_value = sqrt(dm_value); You need to include math.h for sqrt. http://codereview.chromium.org/2885002/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
