LGTM

http://codereview.chromium.org/6658034/diff/2001/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

http://codereview.chromium.org/6658034/diff/2001/src/arm/code-stubs-arm.cc#newcode406
src/arm/code-stubs-arm.cc:406: static void
LoadNumberAsInteger(MacroAssembler* masm,
Maybe this function should have a name that indicate that it will do the
truncate. Also the comment "Floating point value in the 32-bit integer
range will be rounded to an integer." is not totally accurate any more.

http://codereview.chromium.org/6658034/diff/2001/src/arm/code-stubs-arm.cc#newcode638
src/arm/code-stubs-arm.cc:638: CpuFeatures::Scope scope(VFP3);
Should we not support non-vfp here? VFP instructions are only used for
loading the two words of the float as far as I can see.

http://codereview.chromium.org/6658034/diff/2001/src/arm/code-stubs-arm.cc#newcode646
src/arm/code-stubs-arm.cc:646: __ sub(dst, dst, Operand(1023));
1023 -> HeapNumber::kExponentBias

http://codereview.chromium.org/6658034/diff/2001/src/arm/code-stubs-arm.cc#newcode652
src/arm/code-stubs-arm.cc:652: __ sub(dst, dst, Operand(31));
You could combine this sub with the one before subtracting the bias.

http://codereview.chromium.org/6658034/diff/2001/src/arm/code-stubs-arm.cc#newcode653
src/arm/code-stubs-arm.cc:653: __ cmp(dst, Operand(53));
We have HeapNumber::kMantissaBits which is 52.

http://codereview.chromium.org/6658034/diff/2001/src/arm/code-stubs-arm.cc#newcode693
src/arm/code-stubs-arm.cc:693: __ bind(&neg_shift);
neg_shift -> shift_done?

http://codereview.chromium.org/6658034/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to