LGTM with a couple of nits.
http://codereview.chromium.org/7307030/diff/21002/src/arm/stub-cache-arm.cc File src/arm/stub-cache-arm.cc (right): http://codereview.chromium.org/7307030/diff/21002/src/arm/stub-cache-arm.cc#newcode4210 src/arm/stub-cache-arm.cc:4210: __ cmp(scratch, Operand(0x7FFFFFFF)); Can you use kHoleNaNUpper32 here? http://codereview.chromium.org/7307030/diff/21002/src/arm/stub-cache-arm.cc#newcode4360 src/arm/stub-cache-arm.cc:4360: __ ldr(scratch3, FieldMemOperand(value_reg, HeapNumber::kExponentOffset)); Are scratch2 and scratch3 used for anything but exponent and mantissa? If not, those would be better names for them. http://codereview.chromium.org/7307030/diff/21002/src/assembler.cc File src/assembler.cc (right): http://codereview.chromium.org/7307030/diff/21002/src/assembler.cc#newcode50 src/assembler.cc:50: #include "v8.h" This is already included above. http://codereview.chromium.org/7307030/diff/21002/src/ia32/stub-cache-ia32.cc File src/ia32/stub-cache-ia32.cc (right): http://codereview.chromium.org/7307030/diff/21002/src/ia32/stub-cache-ia32.cc#newcode3822 src/ia32/stub-cache-ia32.cc:3822: __ fld_d(FieldOperand(ecx, eax, times_4, FixedDoubleArray::kHeaderSize)); Would it be worth it to check for SSE2 here as well as in the stores and use xmm registers? We should probably see if it makes a difference. http://codereview.chromium.org/7307030/diff/21002/src/ia32/stub-cache-ia32.cc#newcode3830 src/ia32/stub-cache-ia32.cc:3830: __ ffree(); Maybe add a short comment like: Pop double from FPU stack and enter the runtime system to allocate result. http://codereview.chromium.org/7307030/diff/21002/src/ia32/stub-cache-ia32.cc#newcode3931 src/ia32/stub-cache-ia32.cc:3931: __ cmp(FieldOperand(eax, offset), Immediate(0x7ff00000)); You had a comment about this constant in the ARM code. We should probably name this constant as well. http://codereview.chromium.org/7307030/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
