Implement KeyedStoreICs to grow arrays on out-of-bound stores.

Implement KeyedStoreICs to grow arrays on out-of-bound stores.

Supports growing non-COW JSArray by a single element if the backing store has room, and initial allocation of a backing store for the store to index zero of an empty array  to kPreallocatedArrayElements elements (e.g. the [] array literal).

Committed: https://code.google.com/p/v8/source/detail?r=10673

[danno, 2012-02-08 14:32:00]

Some highlights of the change:
* Introduced KeyedAccessGrowMode, which specifies whether an IC supports growing
an array
* Extended ExtraICState to hold both strict mode flag and KeyedAccessGrowMode
* All empty JSArray backing stores (FAST_SMI_ONLY_ELEMENTS, FAST_ELEMENTS,
FAST_DOUBLE_ELEMENTS) are now the same object, the fixed_empty_array.
* Created a fast path for transitioning the ElementsKind of empty arrays in
codegen-XXX.cc
* Extended the existing KeyedStoreIC implementations for FixedArray and
FixedDoubleArray backing stores to handle array growth.

Review guidance:
* Start with the changes in ic.cc to ComputeStub and GetStubKind to handle
elements growing for both the monomorphic and polymorphic stubs.
* Look at the new versions of the generated stub code in stub-cache-XXX.cc

[Jakob Kummerow, 2012-02-09 14:51:24]

I've only taken a cursory glance, and all I've found are three nits :-)

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#newcode990
src/code-stubs.h:990: : is_js_array_(is_js_array),
nit: 4-space indent

http://codereview.chromium.org/9310117/diff/13029/src/objects-debug.cc
File src/objects-debug.cc (right):

http://codereview.chromium.org/9310117/diff/13029/src/objects-debug.cc#newcod...
src/objects-debug.cc:284: || map()->has_fast_smi_only_elements() ||
nit: all operators at the end of the line, please.

http://codereview.chromium.org/9310117/diff/13029/src/stub-cache.h
File src/stub-cache.h (right):

http://codereview.chromium.org/9310117/diff/13029/src/stub-cache.h#newcode707
src/stub-cache.h:707: PropertyType type,
Why this formatting change?

[Vyacheslav Egorov (Chromium), 2012-02-09 16:29:19]

Initial round of comments

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#newc...
src/arm/codegen-arm.cc:126: EMIT_REMEMBERED_SET,
OMIT_REMEMBERED_SET (maps are never in new space).

http://codereview.chromium.org/9310117/diff/13029/src/arm/codegen-arm.cc#newc...
src/arm/codegen-arm.cc:163: EMIT_REMEMBERED_SET,
OMIT_REMEMBERED_SET (maps are never in new space).

http://codereview.chromium.org/9310117/diff/13029/src/arm/codegen-arm.cc#newc...
src/arm/codegen-arm.cc:329: EMIT_REMEMBERED_SET,
OMIT_

http://codereview.chromium.org/9310117/diff/13029/src/ia32/codegen-ia32.cc
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/9310117/diff/13029/src/ia32/codegen-ia32.cc#ne...
src/ia32/codegen-ia32.cc:422: EMIT_REMEMBERED_SET,
map can not be in new space. 

We only need an incremental write barrier here.

http://codereview.chromium.org/9310117/diff/13029/src/ia32/codegen-ia32.cc#ne...
src/ia32/codegen-ia32.cc:474: EMIT_REMEMBERED_SET,
ditto

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...
src/ia32/stub-cache-ia32.cc:3776: if (is_js_array && grow_mode ==
ALLOW_JSARRAY_GROWTH) {
We might have setters on the prototype:


function add(x) { x[x.length] = x.length; }

Array.prototype.__defineSetter__("1", function (v) {
  print("I am setter!");
});

var x = new Array(0);
add(x);
add(x);

I think this setter will fire without this CL (which is correct) but will not
fire with this CL. This seems like yet another "setter on the prototype" compat
problem.

http://codereview.chromium.org/9310117/diff/13029/src/ia32/stub-cache-ia32.cc...
src/ia32/stub-cache-ia32.cc:3823: __ add(FieldOperand(edx,
JSArray::kLengthOffset),
can be actually move of smi 1 instead of addition as length is known to be 0.

[Vyacheslav Egorov (Chromium), 2012-02-10 00:19:18]

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#newc...
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#newc...
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#n...
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#n...
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#n...
src/arm/stub-cache-arm.cc:4353: __ cmp(scratch1, scratch2);
CompareRoot

http://codereview.chromium.org/9310117/diff/13029/src/arm/stub-cache-arm.cc#n...
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#n...
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#n...
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...
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#newc...
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#newc...
src/x64/codegen-x64.cc:257: EMIT_REMEMBERED_SET,
OMIT_

http://codereview.chromium.org/9310117/diff/13029/src/x64/codegen-x64.cc#newc...
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#newc...
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#n...
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#n...
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#n...
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#n...
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#n...
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#n...
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#n...
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#n...
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#n...
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#n...
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-an...
File test/mjsunit/array-store-and-grow.js (right):

http://codereview.chromium.org/9310117/diff/13029/test/mjsunit/array-store-an...
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-an...
test/mjsunit/array-store-and-grow.js:174: array_store_1(a, 0, 0.5);
_9

[danno, 2012-02-10 12:25:34]

Feedback addressed, landing.

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#newc...
src/arm/codegen-arm.cc:100: __ cmp(r4, r5);
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> CompareRoot(r4, Heap::kEmptyFixedArrayRootIndex) ?

Done.

http://codereview.chromium.org/9310117/diff/13029/src/arm/codegen-arm.cc#newc...
src/arm/codegen-arm.cc:126: EMIT_REMEMBERED_SET,
On 2012/02/09 16:29:19, Vyacheslav Egorov wrote:
> OMIT_REMEMBERED_SET (maps are never in new space).

Done.

http://codereview.chromium.org/9310117/diff/13029/src/arm/codegen-arm.cc#newc...
src/arm/codegen-arm.cc:163: EMIT_REMEMBERED_SET,
On 2012/02/09 16:29:19, Vyacheslav Egorov wrote:
> OMIT_REMEMBERED_SET (maps are never in new space).

Done.

http://codereview.chromium.org/9310117/diff/13029/src/arm/codegen-arm.cc#newc...
src/arm/codegen-arm.cc:235: __ cmp(r4, r5);
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> CompareRoot?

Done.

http://codereview.chromium.org/9310117/diff/13029/src/arm/codegen-arm.cc#newc...
src/arm/codegen-arm.cc:329: EMIT_REMEMBERED_SET,
On 2012/02/09 16:29:19, Vyacheslav Egorov wrote:
> OMIT_

Done.

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#n...
src/arm/stub-cache-arm.cc:4210: __ LoadRoot(scratch,
Heap::kHeapNumberMapRootIndex);
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> CompareRoot? 
> 
> also root index is incorrect. should be empty fixed array.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/arm/stub-cache-arm.cc#n...
src/arm/stub-cache-arm.cc:4242: __ add(length_reg, length_reg,
Operand(Smi::FromInt(1)));
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> can write directly 1 into the length field.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/arm/stub-cache-arm.cc#n...
src/arm/stub-cache-arm.cc:4353: __ cmp(scratch1, scratch2);
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> CompareRoot

Done.

http://codereview.chromium.org/9310117/diff/13029/src/arm/stub-cache-arm.cc#n...
src/arm/stub-cache-arm.cc:4364: __ cmp(elements_reg, scratch1);
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> CompareRoot
> 
> also root index is incorrect. should be empty fixed array.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/arm/stub-cache-arm.cc#n...
src/arm/stub-cache-arm.cc:4384: __ str(elements_reg,
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> elements of the backing store are completely uninitialized; if this is
> intentional please add comment.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/arm/stub-cache-arm.cc#n...
src/arm/stub-cache-arm.cc:4391: __ add(length_reg, length_reg,
Operand(Smi::FromInt(1)));
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> store 1 directly without addition.

Done.

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#newcode990
src/code-stubs.h:990: : is_js_array_(is_js_array),
On 2012/02/09 14:51:25, Jakob wrote:
> nit: 4-space indent

Done.

http://codereview.chromium.org/9310117/diff/13029/src/code-stubs.h#newcode996
src/code-stubs.h:996: int multiplier = is_js_array_ ? 1 : 0;
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> is there any particular reason why you don't want to use BitField's here? I
> think it would be much more readable.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/code-stubs.h#newcode1103
src/code-stubs.h:1103: StrictModeBits::encode(strict_mode_);
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> i think grow_mode_ should be embedded into MinorKey.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/ia32/codegen-ia32.cc
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/9310117/diff/13029/src/ia32/codegen-ia32.cc#ne...
src/ia32/codegen-ia32.cc:422: EMIT_REMEMBERED_SET,
On 2012/02/09 16:29:19, Vyacheslav Egorov wrote:
> map can not be in new space. 
> 
> We only need an incremental write barrier here.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/ia32/codegen-ia32.cc#ne...
src/ia32/codegen-ia32.cc:474: EMIT_REMEMBERED_SET,
On 2012/02/09 16:29:19, Vyacheslav Egorov wrote:
> ditto

Done.

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...
src/ia32/stub-cache-ia32.cc:3776: if (is_js_array && grow_mode ==
ALLOW_JSARRAY_GROWTH) {
It turns out the existing elements ICs are very broken  WRT honoring setters,
even if they are set on the prototype before the IC is used. This CL doesn't
make it any worse, but this is totally something we need to fix. 
On 2012/02/09 16:29:19, Vyacheslav Egorov wrote:
> We might have setters on the prototype:
> 
> 
> function add(x) { x[x.length] = x.length; }
> 
> Array.prototype.__defineSetter__("1", function (v) {
>   print("I am setter!");
> });
> 
> var x = new Array(0);
> add(x);
> add(x);
> 
> I think this setter will fire without this CL (which is correct) but will not
> fire with this CL. This seems like yet another "setter on the prototype"
compat
> problem.

http://codereview.chromium.org/9310117/diff/13029/src/ia32/stub-cache-ia32.cc...
src/ia32/stub-cache-ia32.cc:3823: __ add(FieldOperand(edx,
JSArray::kLengthOffset),
On 2012/02/09 16:29:19, Vyacheslav Egorov wrote:
> can be actually move of smi 1 instead of addition as length is known to be 0.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/ia32/stub-cache-ia32.cc...
src/ia32/stub-cache-ia32.cc:3957: __ mov(FieldOperand(edx,
JSObject::kElementsOffset), edi);
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> elements of the backing store are completely uninitialized; if this is
> intentional please add comment.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/objects-debug.cc
File src/objects-debug.cc (right):

http://codereview.chromium.org/9310117/diff/13029/src/objects-debug.cc#newcod...
src/objects-debug.cc:284: || map()->has_fast_smi_only_elements() ||
On 2012/02/09 14:51:25, Jakob wrote:
> nit: all operators at the end of the line, please.

Done.

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) {
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> Consider using BitField to encode and decode extra_ic_state.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/stub-cache.h
File src/stub-cache.h (right):

http://codereview.chromium.org/9310117/diff/13029/src/stub-cache.h#newcode707
src/stub-cache.h:707: PropertyType type,
On 2012/02/09 14:51:25, Jakob wrote:
> Why this formatting change?

Done.

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#newc...
src/x64/codegen-x64.cc:190: __ Cmp(r8,
masm->isolate()->factory()->empty_fixed_array());
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> CompareRoot

Done.

http://codereview.chromium.org/9310117/diff/13029/src/x64/codegen-x64.cc#newc...
src/x64/codegen-x64.cc:257: EMIT_REMEMBERED_SET,
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> OMIT_

Done.

http://codereview.chromium.org/9310117/diff/13029/src/x64/codegen-x64.cc#newc...
src/x64/codegen-x64.cc:303: __ Cmp(r8,
masm->isolate()->factory()->empty_fixed_array());
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> CompareRoot

Done.

http://codereview.chromium.org/9310117/diff/13029/src/x64/codegen-x64.cc#newc...
src/x64/codegen-x64.cc:394: EMIT_REMEMBERED_SET,
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> OMIT

Done.

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#n...
src/x64/stub-cache-x64.cc:3562: __ Cmp(rdi,
masm->isolate()->factory()->empty_fixed_array());
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> CompareRoot

Done.

http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#n...
src/x64/stub-cache-x64.cc:3582: __ Move(rbx,
masm->isolate()->factory()->the_hole_value());
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> LoadRoot

Done.

http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#n...
src/x64/stub-cache-x64.cc:3596: __ SmiAddConstant(FieldOperand(rdx,
JSArray::kLengthOffset),
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> Store 1 directly into field instead of addition.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#n...
src/x64/stub-cache-x64.cc:3602: __ Cmp(FieldOperand(rdi,
HeapObject::kMapOffset),
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> CompareRoot.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#n...
src/x64/stub-cache-x64.cc:3693: __ Cmp(FieldOperand(rax,
HeapObject::kMapOffset),
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> CompareRoot

Done.

http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#n...
src/x64/stub-cache-x64.cc:3701: __ Cmp(rdi,
masm->isolate()->factory()->empty_fixed_array());
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> CompareRoot

Done.

http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#n...
src/x64/stub-cache-x64.cc:3718:
masm->isolate()->factory()->fixed_double_array_map());
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> indentation

Done.

http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#n...
src/x64/stub-cache-x64.cc:3720:
Smi::FromInt(JSArray::kPreallocatedArrayElements));
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> indentation

Done.

http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#n...
src/x64/stub-cache-x64.cc:3723: __ movq(FieldOperand(rdx,
JSObject::kElementsOffset), rdi);
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> elements seem to be uninitialized. if this is intentional please add a
comment.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#n...
src/x64/stub-cache-x64.cc:3728: __ SmiAddConstant(FieldOperand(rdx,
JSArray::kLengthOffset),
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> Store 1 directly instead of add.

Done.

http://codereview.chromium.org/9310117/diff/13029/src/x64/stub-cache-x64.cc#n...
src/x64/stub-cache-x64.cc:3728: __ SmiAddConstant(FieldOperand(rdx,
JSArray::kLengthOffset),
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> Store 1 directly instead of add.

Done.

http://codereview.chromium.org/9310117/diff/13029/test/mjsunit/array-store-an...
File test/mjsunit/array-store-and-grow.js (right):

http://codereview.chromium.org/9310117/diff/13029/test/mjsunit/array-store-an...
test/mjsunit/array-store-and-grow.js:27: 
On 2012/02/10 00:19:18, Vyacheslav Egorov wrote:
> 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);

Done.