http://codereview.chromium.org/2885002/diff/26001/24004 File src/arm/codegen-arm.cc (right):
http://codereview.chromium.org/2885002/diff/26001/24004#newcode4282 src/arm/codegen-arm.cc:4282: // Generates the Math.pow method - currently just calls runtime. On 2010/06/28 12:44:00, Rico wrote:
Remove text after hyphen in comment above
Done. http://codereview.chromium.org/2885002/diff/26001/24004#newcode4294 src/arm/codegen-arm.cc:4294: Label not_minus_half, allocate_return, runtime; On 2010/06/28 12:38:32, Erik Corry wrote:
If you use JumpTarget here instead of Label then you shouldn't have to
handle
the virtual frame manually below.
Changed runtime to a jump target. Kept not_minus_half and allocate_return as labels. http://codereview.chromium.org/2885002/diff/26001/24004#newcode4300 src/arm/codegen-arm.cc:4300: frame_->SpillAllButCopyTOSToR1R0(); On 2010/06/28 12:38:32, Erik Corry wrote:
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.
Done. http://codereview.chromium.org/2885002/diff/26001/24004#newcode4311 src/arm/codegen-arm.cc:4311: __ LoadRoot(r6, Heap::kHeapNumberMapRootIndex); On 2010/06/28 12:38:32, Erik Corry wrote:
Here you should use heap_number_map instead of r6.
Done. http://codereview.chromium.org/2885002/diff/26001/24004#newcode4322 src/arm/codegen-arm.cc:4322: __ b(ne, ¬_minus_half); On 2010/06/28 12:38:32, Erik Corry wrote:
Might as well go straight to runtime here.
Done. http://codereview.chromium.org/2885002/diff/26001/24004#newcode4334 src/arm/codegen-arm.cc:4334: __ mov(scratch2, Operand(0x3ff00000)); On 2010/06/28 12:38:32, Erik Corry wrote:
for a tiny microoptimization reverse the order of these two 'mov's.
Done. http://codereview.chromium.org/2885002/diff/26001/24004#newcode4347 src/arm/codegen-arm.cc:4347: __ b(ne, &runtime); On 2010/06/28 12:38:32, Erik Corry wrote:
You can remove this test if you went straight to runtime above.
Done. http://codereview.chromium.org/2885002/diff/26001/24004#newcode4395 src/arm/codegen-arm.cc:4395: frame()->Dup(); On 2010/06/28 12:38:32, Erik Corry wrote:
I think you can just push it at the point where you call the runtime.
Done. 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)); On 2010/06/28 12:38:32, Erik Corry wrote:
You can do Sbfx and then compare with -1 here. That avoids two loads
to ip. Done. 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 On 2010/06/28 12:38:32, Erik Corry wrote:
This should probably be 1 << 1.
Done. http://codereview.chromium.org/2885002/diff/26001/24007#newcode498 src/arm/macro-assembler-arm.h:498: Register scratch2, On 2010/06/28 12:38:32, Erik Corry wrote:
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.
Done. 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); On 2010/06/28 12:38:32, Erik Corry wrote:
You need to include math.h for sqrt.
Done. http://codereview.chromium.org/2885002/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
