Add bit_field3 to Map objects

Add bit_field3 to Map objects

Reuse instance_descriptor field in the map to store additional flags when there are no descriptors. When descriptors get added to the map, move the flags to the DescriptorArray and access through indirection.

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

[danno, 2011-05-20 10:19:27]

Please review.

[Mads Ager (chromium), 2011-05-23 10:14:51]

LGTM

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

http://codereview.chromium.org/7033024/diff/6001/src/ia32/macro-assembler-ia3...
src/ia32/macro-assembler-ia32.cc:1977: JumpIfNotSmi(descriptors, &not_smi);
Does this accept a Label::kNear argument. This should be a near jump.

http://codereview.chromium.org/7033024/diff/6001/src/objects-inl.h
File src/objects-inl.h (right):

http://codereview.chromium.org/7033024/diff/6001/src/objects-inl.h#newcode1690
src/objects-inl.h:1690: !IsEmpty();
ASSERT(!IsEmpty()) ?

http://codereview.chromium.org/7033024/diff/6001/src/objects-inl.h#newcode2983
src/objects-inl.h:2983: Object* object = READ_FIELD(this,
Doesn't this fit on one line? Ditto for the occurrences below?

http://codereview.chromium.org/7033024/diff/6001/src/objects-inl.h#newcode2996
src/objects-inl.h:2996: 0);
Smi::FromInt(0)

It is the same, but makes it explicit that it is smi tagged.

http://codereview.chromium.org/7033024/diff/6001/src/objects-inl.h#newcode3016
src/objects-inl.h:3016: if (value == HEAP->empty_descriptor_array()) {
You shouldn't have to go through thread local storage here. Doesn't Map have an
isolate() method?

isolate()->heap()->empty_descriptor_array() ?

http://codereview.chromium.org/7033024/diff/6001/src/objects-inl.h#newcode3048
src/objects-inl.h:3048: ASSERT((value & 0x80000000) == 0);  // bit_field3 can
only contain smis
ASSERT(Smi::IsValid(value))

http://codereview.chromium.org/7033024/diff/6001/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/7033024/diff/6001/src/objects.h#newcode2042
src/objects.h:2042: //   [0]: storage for bit_field3 for Map owning this object
(Smi)
Add a TODO(bugnumber) comment about moving this to the map where it belongs.

http://codereview.chromium.org/7033024/diff/6001/src/objects.h#newcode2091
src/objects.h:2091: inline int bit_field3_storage();
I would probably repeat the TODO(bugnumber) comment here.

http://codereview.chromium.org/7033024/diff/6001/src/objects.h#newcode3656
src/objects.h:3656: // Bit field 3.
TODO(bugnumber) comment explaining that this is really in the descriptor array
at this point and that we need to fix that when map pointers are no longer
encoded for GC.

http://codereview.chromium.org/7033024/diff/6001/src/objects.h#newcode3782
src/objects.h:3782: // value the instance descriptor member.
value the -> value of the?

http://codereview.chromium.org/7033024/diff/6001/src/objects.h#newcode3789
src/objects.h:3789: // list.
list -> array

[danno, 2011-06-01 13:15:11]

Feedback integrated and landed.

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

http://codereview.chromium.org/7033024/diff/6001/src/ia32/macro-assembler-ia3...
src/ia32/macro-assembler-ia32.cc:1977: JumpIfNotSmi(descriptors, &not_smi);
There is no Label::xxx parameter supported on the SMI check macros yet. Somebody
(Jakob?) should go back and fix them.
On 2011/05/23 10:14:51, Mads Ager wrote:
> Does this accept a Label::kNear argument. This should be a near jump.

http://codereview.chromium.org/7033024/diff/6001/src/objects-inl.h
File src/objects-inl.h (right):

http://codereview.chromium.org/7033024/diff/6001/src/objects-inl.h#newcode1690
src/objects-inl.h:1690: !IsEmpty();
On 2011/05/23 10:14:51, Mads Ager wrote:
> ASSERT(!IsEmpty()) ?

Done.

http://codereview.chromium.org/7033024/diff/6001/src/objects-inl.h#newcode2983
src/objects-inl.h:2983: Object* object = READ_FIELD(this,
On 2011/05/23 10:14:51, Mads Ager wrote:
> Doesn't this fit on one line? Ditto for the occurrences below?

Done.

http://codereview.chromium.org/7033024/diff/6001/src/objects-inl.h#newcode2996
src/objects-inl.h:2996: 0);
On 2011/05/23 10:14:51, Mads Ager wrote:
> Smi::FromInt(0)
> 
> It is the same, but makes it explicit that it is smi tagged.

Done.

http://codereview.chromium.org/7033024/diff/6001/src/objects-inl.h#newcode3016
src/objects-inl.h:3016: if (value == HEAP->empty_descriptor_array()) {
On 2011/05/23 10:14:51, Mads Ager wrote:
> You shouldn't have to go through thread local storage here. Doesn't Map have
an
> isolate() method?
> 
> isolate()->heap()->empty_descriptor_array() ?

Done.

http://codereview.chromium.org/7033024/diff/6001/src/objects-inl.h#newcode3048
src/objects-inl.h:3048: ASSERT((value & 0x80000000) == 0);  // bit_field3 can
only contain smis
On 2011/05/23 10:14:51, Mads Ager wrote:
> ASSERT(Smi::IsValid(value))

Done.

http://codereview.chromium.org/7033024/diff/6001/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/7033024/diff/6001/src/objects.h#newcode2042
src/objects.h:2042: //   [0]: storage for bit_field3 for Map owning this object
(Smi)
On 2011/05/23 10:14:51, Mads Ager wrote:
> Add a TODO(bugnumber) comment about moving this to the map where it belongs.

Done.

http://codereview.chromium.org/7033024/diff/6001/src/objects.h#newcode2091
src/objects.h:2091: inline int bit_field3_storage();
On 2011/05/23 10:14:51, Mads Ager wrote:
> I would probably repeat the TODO(bugnumber) comment here.

Done.

http://codereview.chromium.org/7033024/diff/6001/src/objects.h#newcode3656
src/objects.h:3656: // Bit field 3.
On 2011/05/23 10:14:51, Mads Ager wrote:
> TODO(bugnumber) comment explaining that this is really in the descriptor array
> at this point and that we need to fix that when map pointers are no longer
> encoded for GC.

Done.

http://codereview.chromium.org/7033024/diff/6001/src/objects.h#newcode3782
src/objects.h:3782: // value the instance descriptor member.
On 2011/05/23 10:14:51, Mads Ager wrote:
> value the -> value of the?

Done.

http://codereview.chromium.org/7033024/diff/6001/src/objects.h#newcode3789
src/objects.h:3789: // list.
On 2011/05/23 10:14:51, Mads Ager wrote:
> list -> array

Done.