Add map check for COW elements to crankshaft array handling code.

Add map check for COW elements to crankshaft array handling code.

BUG=v8:1560
TEST=mjsunit/regress/regress-1560.js

Committed: http://code.google.com/p/v8/source/detail?r=8656

[Jakob Kummerow, 2011-07-14 12:31:58]

Thanks for the bug report. PTAL at the fix.

[danno, 2011-07-14 12:48:43]

LGTM

[Vyacheslav Egorov (Chromium), 2011-07-14 13:03:44]

LGTM with comments addressed.

http://codereview.chromium.org/7366008/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/7366008/diff/1/src/hydrogen.cc#newcode3924
src/hydrogen.cc:3924: HInstruction* elements_cow_map_check = new(zone())
HCheckMap(
I find the name of the variable confusing. We are checking exactly opposite
(that elements are NOT COW.

http://codereview.chromium.org/7366008/diff/1/src/hydrogen.cc#newcode3930
src/hydrogen.cc:3930: AddInstruction(elements_cow_map_check);
This map check is redundant if !is_store.

Can you also extend the regression testcase that tests monomorphic keyed stores
(current testcase tests polymorphic stores)?

http://codereview.chromium.org/7366008/diff/1/src/hydrogen.cc#newcode3933
src/hydrogen.cc:3933: AddInstruction(elements_cow_map_check);
Ditto.

http://codereview.chromium.org/7366008/diff/1/src/hydrogen.cc#newcode4039
src/hydrogen.cc:4039: AddInstruction(new(zone()) HCheckMap(
redundant if !is_store

http://codereview.chromium.org/7366008/diff/1/src/hydrogen.cc#newcode4058
src/hydrogen.cc:4058: AddInstruction(new(zone()) HCheckMap(
This map check is redundant if !is_store

[Jakob Kummerow, 2011-07-14 14:44:49]

Landing.

http://codereview.chromium.org/7366008/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/7366008/diff/1/src/hydrogen.cc#newcode3924
src/hydrogen.cc:3924: HInstruction* elements_cow_map_check = new(zone())
HCheckMap(
On 2011/07/14 13:03:44, Vyacheslav Egorov wrote:
> I find the name of the variable confusing. We are checking exactly opposite
> (that elements are NOT COW.

Done, sort of: I've removed the variable completely.

http://codereview.chromium.org/7366008/diff/1/src/hydrogen.cc#newcode3930
src/hydrogen.cc:3930: AddInstruction(elements_cow_map_check);
On 2011/07/14 13:03:44, Vyacheslav Egorov wrote:
> This map check is redundant if !is_store.

Done.

> Can you also extend the regression testcase that tests monomorphic keyed
stores
> (current testcase tests polymorphic stores)? 

Done.

http://codereview.chromium.org/7366008/diff/1/src/hydrogen.cc#newcode3933
src/hydrogen.cc:3933: AddInstruction(elements_cow_map_check);
On 2011/07/14 13:03:44, Vyacheslav Egorov wrote:
> Ditto.

Done.

http://codereview.chromium.org/7366008/diff/1/src/hydrogen.cc#newcode4039
src/hydrogen.cc:4039: AddInstruction(new(zone()) HCheckMap(
On 2011/07/14 13:03:44, Vyacheslav Egorov wrote:
> redundant if !is_store

Done.

http://codereview.chromium.org/7366008/diff/1/src/hydrogen.cc#newcode4058
src/hydrogen.cc:4058: AddInstruction(new(zone()) HCheckMap(
On 2011/07/14 13:03:44, Vyacheslav Egorov wrote:
> This map check is redundant if !is_store

Done.