lgtm
http://codereview.chromium.org/9310117/diff/13029/src/arm/codegen-arm.cc File src/arm/codegen-arm.cc (right): http://codereview.chromium.org/9310117/diff/13029/src/arm/codegen-arm.cc#newcode100 src/arm/codegen-arm.cc:100: __ cmp(r4, r5); CompareRoot(r4, Heap::kEmptyFixedArrayRootIndex) ? http://codereview.chromium.org/9310117/diff/13029/src/arm/codegen-arm.cc#newcode235 src/arm/codegen-arm.cc:235: __ cmp(r4, r5); CompareRoot? http://codereview.chromium.org/9310117/diff/13029/src/arm/stub-cache-arm.cc File src/arm/stub-cache-arm.cc (right): http://codereview.chromium.org/9310117/diff/13029/src/arm/stub-cache-arm.cc#newcode4210 src/arm/stub-cache-arm.cc:4210: __ LoadRoot(scratch, Heap::kHeapNumberMapRootIndex); CompareRoot? also root index is incorrect. should be empty fixed array. http://codereview.chromium.org/9310117/diff/13029/src/arm/stub-cache-arm.cc#newcode4242 src/arm/stub-cache-arm.cc:4242: __ add(length_reg, length_reg, Operand(Smi::FromInt(1))); can write directly 1 into the length field. http://codereview.chromium.org/9310117/diff/13029/src/arm/stub-cache-arm.cc#newcode4353 src/arm/stub-cache-arm.cc:4353: __ cmp(scratch1, scratch2); CompareRoot http://codereview.chromium.org/9310117/diff/13029/src/arm/stub-cache-arm.cc#newcode4364 src/arm/stub-cache-arm.cc:4364: __ cmp(elements_reg, scratch1); CompareRoot also root index is incorrect. should be empty fixed array. http://codereview.chromium.org/9310117/diff/13029/src/arm/stub-cache-arm.cc#newcode4384 src/arm/stub-cache-arm.cc:4384: __ str(elements_reg, elements of the backing store are completely uninitialized; if this is intentional please add comment. http://codereview.chromium.org/9310117/diff/13029/src/arm/stub-cache-arm.cc#newcode4391 src/arm/stub-cache-arm.cc:4391: __ add(length_reg, length_reg, Operand(Smi::FromInt(1))); store 1 directly without addition. http://codereview.chromium.org/9310117/diff/13029/src/code-stubs.h File src/code-stubs.h (right): http://codereview.chromium.org/9310117/diff/13029/src/code-stubs.h#newcode996 src/code-stubs.h:996: int multiplier = is_js_array_ ? 1 : 0; is there any particular reason why you don't want to use BitField's here? I think it would be much more readable. http://codereview.chromium.org/9310117/diff/13029/src/code-stubs.h#newcode1103 src/code-stubs.h:1103: StrictModeBits::encode(strict_mode_); i think grow_mode_ should be embedded into MinorKey. http://codereview.chromium.org/9310117/diff/13029/src/ia32/stub-cache-ia32.cc File src/ia32/stub-cache-ia32.cc (right): http://codereview.chromium.org/9310117/diff/13029/src/ia32/stub-cache-ia32.cc#newcode3957 src/ia32/stub-cache-ia32.cc:3957: __ mov(FieldOperand(edx, JSObject::kElementsOffset), edi); elements of the backing store are completely uninitialized; if this is intentional please add comment. http://codereview.chromium.org/9310117/diff/13029/src/objects.h File src/objects.h (right): http://codereview.chromium.org/9310117/diff/13029/src/objects.h#newcode4226 src/objects.h:4226: static inline StrictModeFlag GetStrictMode(ExtraICState extra_ic_state) { Consider using BitField to encode and decode extra_ic_state. http://codereview.chromium.org/9310117/diff/13029/src/x64/codegen-x64.cc File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/9310117/diff/13029/src/x64/codegen-x64.cc#newcode190 src/x64/codegen-x64.cc:190: __ Cmp(r8, masm->isolate()->factory()->empty_fixed_array()); CompareRoot http://codereview.chromium.org/9310117/diff/13029/src/x64/codegen-x64.cc#newcode257 src/x64/codegen-x64.cc:257: EMIT_REMEMBERED_SET, OMIT_ http://codereview.chromium.org/9310117/diff/13029/src/x64/codegen-x64.cc#newcode303 src/x64/codegen-x64.cc:303: __ Cmp(r8, masm->isolate()->factory()->empty_fixed_array()); CompareRoot http://codereview.chromium.org/9310117/diff/13029/src/x64/codegen-x64.cc#newcode394 src/x64/codegen-x64.cc:394: EMIT_REMEMBERED_SET, OMIT http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc File src/x64/stub-cache-x64.cc (right): http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#newcode3562 src/x64/stub-cache-x64.cc:3562: __ Cmp(rdi, masm->isolate()->factory()->empty_fixed_array()); CompareRoot http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#newcode3582 src/x64/stub-cache-x64.cc:3582: __ Move(rbx, masm->isolate()->factory()->the_hole_value()); LoadRoot http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#newcode3596 src/x64/stub-cache-x64.cc:3596: __ SmiAddConstant(FieldOperand(rdx, JSArray::kLengthOffset), Store 1 directly into field instead of addition. http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#newcode3602 src/x64/stub-cache-x64.cc:3602: __ Cmp(FieldOperand(rdi, HeapObject::kMapOffset), CompareRoot. http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#newcode3693 src/x64/stub-cache-x64.cc:3693: __ Cmp(FieldOperand(rax, HeapObject::kMapOffset), CompareRoot http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#newcode3701 src/x64/stub-cache-x64.cc:3701: __ Cmp(rdi, masm->isolate()->factory()->empty_fixed_array()); CompareRoot http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#newcode3718 src/x64/stub-cache-x64.cc:3718: masm->isolate()->factory()->fixed_double_array_map()); indentation http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#newcode3720 src/x64/stub-cache-x64.cc:3720: Smi::FromInt(JSArray::kPreallocatedArrayElements)); indentation http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#newcode3723 src/x64/stub-cache-x64.cc:3723: __ movq(FieldOperand(rdx, JSObject::kElementsOffset), rdi); elements seem to be uninitialized. if this is intentional please add a comment. http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#newcode3728 src/x64/stub-cache-x64.cc:3728: __ SmiAddConstant(FieldOperand(rdx, JSArray::kLengthOffset), Store 1 directly instead of add. http://codereview.chromium.org/9310117/diff/13029/test/mjsunit/array-store-and-grow.js File test/mjsunit/array-store-and-grow.js (right): http://codereview.chromium.org/9310117/diff/13029/test/mjsunit/array-store-and-grow.js#newcode27 test/mjsunit/array-store-and-grow.js:27: consider adding test that checks COW handling. like function makeCOW() { return [1,2,3]; } var cow1 = makeCOW(), cow2 = makeCOW(); store(cow1); check_that_cow_is_fine(cow2); http://codereview.chromium.org/9310117/diff/13029/test/mjsunit/array-store-and-grow.js#newcode174 test/mjsunit/array-store-and-grow.js:174: array_store_1(a, 0, 0.5); _9 http://codereview.chromium.org/9310117/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
