Add complete ElementKind information directly to Map for objects with elements.

Add complete ElementsKind information directly to Map for objects with elements.

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

[danno, 2011-05-27 11:39:13]

please review

[Mads Ager (chromium), 2011-05-30 11:14:24]

LGTM with a couple of nits.

http://codereview.chromium.org/6966041/diff/2002/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/6966041/diff/2002/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.cc:1683: cmp(scratch,
Operand(Map::kMaximumBitField2FastElementValue));
Can we add some sort of STATIC_ASSERT here that will trigger if we change the
order of stuff so that it will no longer be safe to only check the top value?

Something like FAST_ELEMENTS == 0?

http://codereview.chromium.org/6966041/diff/2002/src/ia32/ic-ia32.cc
File src/ia32/ic-ia32.cc (right):

http://codereview.chromium.org/6966041/diff/2002/src/ia32/ic-ia32.cc#newcode487
src/ia32/ic-ia32.cc:487: // Check if receiver's map to see if it has fast
elements.
if receiver's -> the receiver's

http://codereview.chromium.org/6966041/diff/2002/src/ia32/macro-assembler-ia3...
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/6966041/diff/2002/src/ia32/macro-assembler-ia3...
src/ia32/macro-assembler-ia32.cc:285: // Check the "has fast elements" bit in
the receiver's map.
Remove comment here? Seems redundant with the comment in the header file?

http://codereview.chromium.org/6966041/diff/2002/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/6966041/diff/2002/src/objects.h#newcode3763
src/objects.h:3763: (elements_kind << kElementsKindFirstBit));
Rename kElementsKindFirstBit to kElementsKindShift? We use the pair of "mask"
and "shift" in many places in the system.

http://codereview.chromium.org/6966041/diff/2002/src/objects.h#newcode4002
src/objects.h:4002: // No bits can be used between kElementsKindFirstBit and
kElementsKindLastBit
Since the elements kind is using the remaining bits, can we get away with just
having kElementsKindShift of 4 and then simplifying the constants defined below?
I don't see how to free up any of these bits in any case, so I'm not sure it
adds much to have the explicit upper bound?

[danno, 2011-06-01 22:29:54]

feedback addressed and will commit

http://codereview.chromium.org/6966041/diff/2002/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/6966041/diff/2002/src/arm/macro-assembler-arm....
src/arm/macro-assembler-arm.cc:1683: cmp(scratch,
Operand(Map::kMaximumBitField2FastElementValue));
On 2011/05/30 11:14:24, Mads Ager wrote:
> Can we add some sort of STATIC_ASSERT here that will trigger if we change the
> order of stuff so that it will no longer be safe to only check the top value?
> 
> Something like FAST_ELEMENTS == 0?

Done.

http://codereview.chromium.org/6966041/diff/2002/src/ia32/ic-ia32.cc
File src/ia32/ic-ia32.cc (right):

http://codereview.chromium.org/6966041/diff/2002/src/ia32/ic-ia32.cc#newcode487
src/ia32/ic-ia32.cc:487: // Check if receiver's map to see if it has fast
elements.
On 2011/05/30 11:14:24, Mads Ager wrote:
> if receiver's -> the receiver's

Done.

http://codereview.chromium.org/6966041/diff/2002/src/ia32/macro-assembler-ia3...
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/6966041/diff/2002/src/ia32/macro-assembler-ia3...
src/ia32/macro-assembler-ia32.cc:285: // Check the "has fast elements" bit in
the receiver's map.
On 2011/05/30 11:14:24, Mads Ager wrote:
> Remove comment here? Seems redundant with the comment in the header file?

Done.

http://codereview.chromium.org/6966041/diff/2002/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/6966041/diff/2002/src/objects.h#newcode3763
src/objects.h:3763: (elements_kind << kElementsKindFirstBit));
On 2011/05/30 11:14:24, Mads Ager wrote:
> Rename kElementsKindFirstBit to kElementsKindShift? We use the pair of "mask"
> and "shift" in many places in the system.

Done.

http://codereview.chromium.org/6966041/diff/2002/src/objects.h#newcode4002
src/objects.h:4002: // No bits can be used between kElementsKindFirstBit and
kElementsKindLastBit
On 2011/05/30 11:14:24, Mads Ager wrote:
> Since the elements kind is using the remaining bits, can we get away with just
> having kElementsKindShift of 4 and then simplifying the constants defined
below?
> I don't see how to free up any of these bits in any case, so I'm not sure it
> adds much to have the explicit upper bound?

Done.