A few small comments. I haven't tested the patch yet. How does it behave on ARM?
Regards, Alexandre http://codereview.chromium.org/7024047/diff/8001/src/arm/code-stubs-arm.cc File src/arm/code-stubs-arm.cc (right): http://codereview.chromium.org/7024047/diff/8001/src/arm/code-stubs-arm.cc#newcode3956 src/arm/code-stubs-arm.cc:3956: __ add(r3, r3, Operand(r2, LSL, 1)); Here and below, '1' standing for 'kPointerSizeLog2 - kSmiTagSize' in shift operations. Should we use the verbose description? http://codereview.chromium.org/7024047/diff/8001/src/arm/code-stubs-arm.cc#newcode4017 src/arm/code-stubs-arm.cc:4017: __ add(r1, r1, Operand(FixedArray::kHeaderSize)); This add and the following one could be merged into only one instruction. http://codereview.chromium.org/7024047/diff/8001/src/arm/code-stubs-arm.cc#newcode4114 src/arm/code-stubs-arm.cc:4114: __ jmp(¶meters_test); I don't know how performance sensitive this loop is, but I think it could be optimized to something like this: Label loop, end; cmp r6, 0 beq end // Setup. // Use r6 as the counter, already shifted to kPointerSize. mov r6, r6 LSL 1 // Compute base addresses of stores. (extracting loop invariant code) add r5, r4, kParameterMapHeaderSize - kHeapObjectTag add r3, r3, FixedArray::kHeaderSize - kHeapObjectTag bind(loop); subs r6, r6, kPointerSize // Decrement the counter and set the flags. // Stores. str r1, [r5, r6] add r1, r1, Smi::FromInt(1) str r7, [r3, r6] bgt, loop // Restore address of backing store. sub r3, r3, FixedArray::kHeaderSize - kHeapObjectTag bind(end); http://codereview.chromium.org/7024047/diff/8001/src/arm/code-stubs-arm.cc#newcode4138 src/arm/code-stubs-arm.cc:4138: __ mov(r1, r9); If necessary I think this loop could also be refactored. - If r4 is not used outside the loop the test to enter the loop can be moved at the very beginning. - Using r1 as a counter - aligned on kPointerSize (with a 'sub' setting flags) - 'r1 LSL 1' shift can be moved to the initial 'mov(r1, r9)' http://codereview.chromium.org/7024047/diff/8001/src/arm/code-stubs-arm.cc#newcode4144 src/arm/code-stubs-arm.cc:4144: __ sub(r4, r4, Operand(kPointerSize)); The sub can be merged with the ldr using NegPreIndex. http://codereview.chromium.org/7024047/diff/8001/src/arm/code-stubs-arm.cc#newcode4239 src/arm/code-stubs-arm.cc:4239: __ mov(r1, Operand(r1, LSR, kSmiTagSize)); If r1 is only used as a counter and not used later there is no need to untag it. It can be decremented by Smi::FromInt(1) instead. http://codereview.chromium.org/7024047/diff/8001/src/arm/code-stubs-arm.cc#newcode4252 src/arm/code-stubs-arm.cc:4252: __ cmp(r1, Operand(0, RelocInfo::NONE)); Could merge the 'sub' and 'cmp' instructions into a 'subs'. http://codereview.chromium.org/7024047/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
