Plumbing changes to merge various element kind implementaions.

Dispatch on ElementsKind rather than ExternalArrayType when generating ICs and Crankshaft code for many element operations. This is preparation to be able to share more code in the various element accessor implementations.

Merge logic to maintain external array and fast element stub caches.

BUG=none
TEST=none

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

[danno, 2011-06-09 12:58:42]

Please review.

Reviewer guidance:

Most of the changes are purely mechanical, However, there are two functionality
changes to keep an eye out for. 

First, in the stub-cache* implementation files, the caching login for fast
elements and external elements has been merged. 

Second, the ExternalArrayType doesn't need to get explicitly stashed away
out-of-band to preserve type information from the ICs, since the ElementKind is
stored directly in the map.

[Mads Ager (chromium), 2011-06-09 14:15:45]

http://codereview.chromium.org/7112010/diff/2014/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

http://codereview.chromium.org/7112010/diff/2014/src/arm/lithium-arm.cc#newco...
src/arm/lithium-arm.cc:2002: bool val_is_temp_register = elements_kind ==
(elements_kind == A) || (elements_kind == B)

?

http://codereview.chromium.org/7112010/diff/2014/src/arm/stub-cache-arm.cc
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/7112010/diff/2014/src/arm/stub-cache-arm.cc#ne...
src/arm/stub-cache-arm.cc:3435: case JSObject::EXTERNAL_PIXEL_ELEMENTS:
Are pixel values signed? I didn't think so. It seems that pixel arrays were not
handled here before. Can this just go in the unhandled part?

http://codereview.chromium.org/7112010/diff/2014/src/arm/stub-cache-arm.cc#ne...
src/arm/stub-cache-arm.cc:3518: case JSObject::EXTERNAL_DOUBLE_ELEMENTS:
Something is off with the indentation I think?

http://codereview.chromium.org/7112010/diff/2014/src/heap.h
File src/heap.h (right):

http://codereview.chromium.org/7112010/diff/2014/src/heap.h#newcode138
src/heap.h:138: #define SYMBOL_LIST(V)                                          
       \
Is there any actual change here?

http://codereview.chromium.org/7112010/diff/2014/src/lithium.cc
File src/lithium.cc (right):

http://codereview.chromium.org/7112010/diff/2014/src/lithium.cc#newcode187
src/lithium.cc:187: return kPointerSizeLog2;
For DoubleElements you should multiply by the size of a double and not by the
size of a pointer, right? Or am I missing something in the way this is used?

[Jakob Kummerow, 2011-06-09 14:26:40]

Some of my comments are dupes of Mads' comments; I couldn't be bothered to
filter them out :-)

http://codereview.chromium.org/7112010/diff/2014/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

http://codereview.chromium.org/7112010/diff/2014/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:2637: } else  {  // i.e. array_type ==
kExternalDoubleArray
please update this comment.

http://codereview.chromium.org/7112010/diff/2014/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:3453: } else {  // i.e. array_type ==
kExternalDoubleArray
please update this comment.

http://codereview.chromium.org/7112010/diff/2014/src/arm/stub-cache-arm.cc
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/7112010/diff/2014/src/arm/stub-cache-arm.cc#ne...
src/arm/stub-cache-arm.cc:3435: case JSObject::EXTERNAL_PIXEL_ELEMENTS:
really?

http://codereview.chromium.org/7112010/diff/2014/src/heap.h
File src/heap.h (right):

http://codereview.chromium.org/7112010/diff/2014/src/heap.h#newcode138
src/heap.h:138: #define SYMBOL_LIST(V)                                          
       \
what exactly is the change here?

http://codereview.chromium.org/7112010/diff/2014/src/lithium.cc
File src/lithium.cc (right):

http://codereview.chromium.org/7112010/diff/2014/src/lithium.cc#newcode185
src/lithium.cc:185: case JSObject::FAST_DOUBLE_ELEMENTS:
FAST_DOUBLE_ELEMENTS should return 3

http://codereview.chromium.org/7112010/diff/2014/src/type-info.cc
File src/type-info.cc (right):

http://codereview.chromium.org/7112010/diff/2014/src/type-info.cc#newcode174
src/type-info.cc:174: JSObject::ElementsKind
TypeFeedbackOracle::GetKeyedLoadElementsKind(
See the comment in type-info.h -- I think you can remove
GetKeyed{Load,Store}ElementsKind().

http://codereview.chromium.org/7112010/diff/2014/src/type-info.h
File src/type-info.h (right):

http://codereview.chromium.org/7112010/diff/2014/src/type-info.h#newcode231
src/type-info.h:231: JSObject::ElementsKind GetKeyedLoadElementsKind(Property*
expr);
I believe these two methods can be removed completely. AFAICS they've only been
used in ast.cc, and you've removed those occurrences.

[danno, 2011-06-09 15:03:27]

Feedback addressed, please take another look.

http://codereview.chromium.org/7112010/diff/2014/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

http://codereview.chromium.org/7112010/diff/2014/src/arm/lithium-arm.cc#newco...
src/arm/lithium-arm.cc:2002: bool val_is_temp_register = elements_kind ==
On 2011/06/09 14:15:45, Mads Ager wrote:
> (elements_kind == A) || (elements_kind == B)
> 
> ?

Done.

http://codereview.chromium.org/7112010/diff/2014/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

http://codereview.chromium.org/7112010/diff/2014/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:2637: } else  {  // i.e. array_type ==
kExternalDoubleArray
On 2011/06/09 14:26:40, jkummerow wrote:
> please update this comment.

Done.

http://codereview.chromium.org/7112010/diff/2014/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:3453: } else {  // i.e. array_type ==
kExternalDoubleArray
On 2011/06/09 14:26:40, jkummerow wrote:
> please update this comment.

Done.

http://codereview.chromium.org/7112010/diff/2014/src/arm/stub-cache-arm.cc
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/7112010/diff/2014/src/arm/stub-cache-arm.cc#ne...
src/arm/stub-cache-arm.cc:3435: case JSObject::EXTERNAL_PIXEL_ELEMENTS:
On 2011/06/09 14:15:45, Mads Ager wrote:
> Are pixel values signed? I didn't think so. It seems that pixel arrays were
not
> handled here before. Can this just go in the unhandled part?

Done.

http://codereview.chromium.org/7112010/diff/2014/src/arm/stub-cache-arm.cc#ne...
src/arm/stub-cache-arm.cc:3435: case JSObject::EXTERNAL_PIXEL_ELEMENTS:
On 2011/06/09 14:26:40, jkummerow wrote:
> really?

Done.

http://codereview.chromium.org/7112010/diff/2014/src/arm/stub-cache-arm.cc#ne...
src/arm/stub-cache-arm.cc:3518: case JSObject::EXTERNAL_DOUBLE_ELEMENTS:
On 2011/06/09 14:15:45, Mads Ager wrote:
> Something is off with the indentation I think?

Done.

http://codereview.chromium.org/7112010/diff/2014/src/heap.h
File src/heap.h (right):

http://codereview.chromium.org/7112010/diff/2014/src/heap.h#newcode138
src/heap.h:138: #define SYMBOL_LIST(V)                                          
       \
On 2011/06/09 14:15:45, Mads Ager wrote:
> Is there any actual change here?

Done. Removed non-relevant white-space changes.

http://codereview.chromium.org/7112010/diff/2014/src/heap.h#newcode138
src/heap.h:138: #define SYMBOL_LIST(V)                                          
       \
On 2011/06/09 14:26:40, jkummerow wrote:
> what exactly is the change here?

Done.

http://codereview.chromium.org/7112010/diff/2014/src/lithium.cc
File src/lithium.cc (right):

http://codereview.chromium.org/7112010/diff/2014/src/lithium.cc#newcode185
src/lithium.cc:185: case JSObject::FAST_DOUBLE_ELEMENTS:
On 2011/06/09 14:26:40, jkummerow wrote:
> FAST_DOUBLE_ELEMENTS should return 3

Done.

http://codereview.chromium.org/7112010/diff/2014/src/lithium.cc#newcode187
src/lithium.cc:187: return kPointerSizeLog2;
Not used yet, but you're right,

On 2011/06/09 14:15:45, Mads Ager wrote:
> For DoubleElements you should multiply by the size of a double and not by the
> size of a pointer, right? Or am I missing something in the way this is used?

http://codereview.chromium.org/7112010/diff/2014/src/type-info.cc
File src/type-info.cc (right):

http://codereview.chromium.org/7112010/diff/2014/src/type-info.cc#newcode174
src/type-info.cc:174: JSObject::ElementsKind
TypeFeedbackOracle::GetKeyedLoadElementsKind(
On 2011/06/09 14:26:40, jkummerow wrote:
> See the comment in type-info.h -- I think you can remove
> GetKeyed{Load,Store}ElementsKind().

Done.

http://codereview.chromium.org/7112010/diff/2014/src/type-info.h
File src/type-info.h (right):

http://codereview.chromium.org/7112010/diff/2014/src/type-info.h#newcode231
src/type-info.h:231: JSObject::ElementsKind GetKeyedLoadElementsKind(Property*
expr);
On 2011/06/09 14:26:40, jkummerow wrote:
> I believe these two methods can be removed completely. AFAICS they've only
been
> used in ast.cc, and you've removed those occurrences.

Done.

[Jakob Kummerow, 2011-06-09 15:12:14]

LGTM