- Add a size field to the header.

- Add a size field to the header.
- Make use of the field when available to avoid accessing the class.
Committed: https://code.google.com/p/dart/source/detail?r=2936

[turnidge, 2012-01-04 01:06:17]

DBC -turnidge

http://codereview.chromium.org/9072011/diff/12/vm/freelist.h
File vm/freelist.h (right):

http://codereview.chromium.org/9072011/diff/12/vm/freelist.h#newcode26
vm/freelist.h:26: return reinterpret_cast<FreeListElement*>(next_ ^ 1);
Should this use the constant RawObject::kFreeBit?

http://codereview.chromium.org/9072011/diff/12/vm/freelist.h#newcode32
vm/freelist.h:32: next_ = addr | 1;
Same as above.

[siva, 2012-01-04 01:56:03]

lgtm

http://codereview.chromium.org/9072011/diff/12/vm/assembler_macros_ia32.cc
File vm/assembler_macros_ia32.cc (right):

http://codereview.chromium.org/9072011/diff/12/vm/assembler_macros_ia32.cc#ne...
vm/assembler_macros_ia32.cc:49: class_reg);
ASSERT(instance_size <= RawObject::SizeTag::kMaxSizeTag);

http://codereview.chromium.org/9072011/diff/12/vm/assembler_macros_x64.cc
File vm/assembler_macros_x64.cc (right):

http://codereview.chromium.org/9072011/diff/12/vm/assembler_macros_x64.cc#new...
vm/assembler_macros_x64.cc:53: class_reg);
ASSERT(instance_size <= RawObject::SizeTag::kMaxSizeTag);

http://codereview.chromium.org/9072011/diff/12/vm/intrinsifier_ia32.cc
File vm/intrinsifier_ia32.cc (right):

http://codereview.chromium.org/9072011/diff/12/vm/intrinsifier_ia32.cc#newcod...
vm/intrinsifier_ia32.cc:138: __ leal(EDI, Address(EDI, TIMES_2, fixed_size)); 
// EDX is Smi.
EDI is Smi?

Also if you moved this block of code to initialize tags above the section that
stores the class value, EDI would not have been clobbered and you could save on
these two instructions which are setting up EDI again.

http://codereview.chromium.org/9072011/diff/12/vm/intrinsifier_ia32.cc#newcod...
vm/intrinsifier_ia32.cc:142: Immediate(RawObject::SizeTag::kMaxSizeTag));
Can this fit in one line?

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc
File vm/stub_code_ia32.cc (right):

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode690
vm/stub_code_ia32.cc:690: Immediate(RawObject::SizeTag::kMaxSizeTag));
Can't this be in one line?

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode1013
vm/stub_code_ia32.cc:1013: Immediate(RawObject::SizeTag::kMaxSizeTag));
Can't this be in one line?

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode1022
vm/stub_code_ia32.cc:1022: __ Bind(&done);
Can this piece of code which is repeated here and above be factored into a
static method:
GenerateStoreTags(Assembler* assembler, Register size);

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode1157
vm/stub_code_ia32.cc:1157: // Set the tags.
ASSERT(type_args_size <= RawObject::SizeTag::kMaxSizeTag);

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode1177
vm/stub_code_ia32.cc:1177: // Set the tags.
ASSERT(instance_size <= RawObject::SizeTag::kMaxSizeTag);

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode1327
vm/stub_code_ia32.cc:1327: // Set the tags.
ASSERT(closure_size <= RawObject::SizeTag::kMaxSizeTag);

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode1352
vm/stub_code_ia32.cc:1352: // Set the tags.
ASSERT(context_size <= RawObject::SizeTag::kMaxSizeTag);

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_x64.cc
File vm/stub_code_x64.cc (right):

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_x64.cc#newcode637
vm/stub_code_x64.cc:637: Immediate(RawObject::SizeTag::kMaxSizeTag));
Can this fit in one line?

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_x64.cc#newcode917
vm/stub_code_x64.cc:917: // Set the tags.
ASSERT(type_args_size <= RawObject::SizeTag::kMaxSizeTag);

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_x64.cc#newcode937
vm/stub_code_x64.cc:937: // Set the tags.
ASSERT(instance_size <= RawObject::SizeTag::kMaxSizeTag);

[Ivan Posva, 2012-01-04 07:59:41]

http://codereview.chromium.org/9072011/diff/12/vm/assembler_macros_ia32.cc
File vm/assembler_macros_ia32.cc (right):

http://codereview.chromium.org/9072011/diff/12/vm/assembler_macros_ia32.cc#ne...
vm/assembler_macros_ia32.cc:49: class_reg);
On 2012/01/04 01:56:03, asiva wrote:
> ASSERT(instance_size <= RawObject::SizeTag::kMaxSizeTag);

This assertion would be wrong. RawObject::SizeTag::encode does the right thing
when instance_size is larger than RawObject::SizeTag::kMaxSizeTag. It sets the
tag to 0.

http://codereview.chromium.org/9072011/diff/12/vm/assembler_macros_x64.cc
File vm/assembler_macros_x64.cc (right):

http://codereview.chromium.org/9072011/diff/12/vm/assembler_macros_x64.cc#new...
vm/assembler_macros_x64.cc:53: class_reg);
On 2012/01/04 01:56:03, asiva wrote:
> ASSERT(instance_size <= RawObject::SizeTag::kMaxSizeTag);

ditto

http://codereview.chromium.org/9072011/diff/12/vm/freelist.h
File vm/freelist.h (right):

http://codereview.chromium.org/9072011/diff/12/vm/freelist.h#newcode26
vm/freelist.h:26: return reinterpret_cast<FreeListElement*>(next_ ^ 1);
On 2012/01/04 01:06:17, turnidge wrote:
> Should this use the constant RawObject::kFreeBit?

Done. In any case this use here should only be temporary.

http://codereview.chromium.org/9072011/diff/12/vm/freelist.h#newcode32
vm/freelist.h:32: next_ = addr | 1;
On 2012/01/04 01:06:17, turnidge wrote:
> Same as above.

ditto.

http://codereview.chromium.org/9072011/diff/12/vm/intrinsifier_ia32.cc
File vm/intrinsifier_ia32.cc (right):

http://codereview.chromium.org/9072011/diff/12/vm/intrinsifier_ia32.cc#newcod...
vm/intrinsifier_ia32.cc:138: __ leal(EDI, Address(EDI, TIMES_2, fixed_size)); 
// EDX is Smi.
On 2012/01/04 01:56:03, asiva wrote:
> EDI is Smi?
> 
> Also if you moved this block of code to initialize tags above the section that
> stores the class value, EDI would not have been clobbered and you could save
on
> these two instructions which are setting up EDI again. 

Rearranged setting of class and tags to avoid recalculating allocation size.

http://codereview.chromium.org/9072011/diff/12/vm/intrinsifier_ia32.cc#newcod...
vm/intrinsifier_ia32.cc:142: Immediate(RawObject::SizeTag::kMaxSizeTag));
On 2012/01/04 01:56:03, asiva wrote:
> Can this fit in one line?

Done.

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc
File vm/stub_code_ia32.cc (right):

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode690
vm/stub_code_ia32.cc:690: Immediate(RawObject::SizeTag::kMaxSizeTag));
On 2012/01/04 01:56:03, asiva wrote:
> Can't this be in one line?

Done.

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode1013
vm/stub_code_ia32.cc:1013: Immediate(RawObject::SizeTag::kMaxSizeTag));
On 2012/01/04 01:56:03, asiva wrote:
> Can't this be in one line?

Done.

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode1022
vm/stub_code_ia32.cc:1022: __ Bind(&done);
On 2012/01/04 01:56:03, asiva wrote:
> Can this piece of code which is repeated here and above be factored into a
> static method:
> GenerateStoreTags(Assembler* assembler, Register size);

We should evaluate that once we are done with setting up all of the tag bits.

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode1157
vm/stub_code_ia32.cc:1157: // Set the tags.
On 2012/01/04 01:56:03, asiva wrote:
> ASSERT(type_args_size <= RawObject::SizeTag::kMaxSizeTag);

ditto

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode1177
vm/stub_code_ia32.cc:1177: // Set the tags.
On 2012/01/04 01:56:03, asiva wrote:
> ASSERT(instance_size <= RawObject::SizeTag::kMaxSizeTag);

ditto

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode1327
vm/stub_code_ia32.cc:1327: // Set the tags.
On 2012/01/04 01:56:03, asiva wrote:
> ASSERT(closure_size <= RawObject::SizeTag::kMaxSizeTag);

ditto.

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_ia32.cc#newcode1352
vm/stub_code_ia32.cc:1352: // Set the tags.
On 2012/01/04 01:56:03, asiva wrote:
> ASSERT(context_size <= RawObject::SizeTag::kMaxSizeTag);

ditto.

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_x64.cc
File vm/stub_code_x64.cc (right):

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_x64.cc#newcode637
vm/stub_code_x64.cc:637: Immediate(RawObject::SizeTag::kMaxSizeTag));
On 2012/01/04 01:56:03, asiva wrote:
> Can this fit in one line?

Done.

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_x64.cc#newcode917
vm/stub_code_x64.cc:917: // Set the tags.
On 2012/01/04 01:56:03, asiva wrote:
> ASSERT(type_args_size <= RawObject::SizeTag::kMaxSizeTag);

ditto.

http://codereview.chromium.org/9072011/diff/12/vm/stub_code_x64.cc#newcode937
vm/stub_code_x64.cc:937: // Set the tags.
On 2012/01/04 01:56:03, asiva wrote:
> ASSERT(instance_size <= RawObject::SizeTag::kMaxSizeTag);

ditto.

[regis, 2012-01-10 20:27:48]

http://codereview.chromium.org/9072011/diff/10001/vm/stub_code_x64.cc
File vm/stub_code_x64.cc (right):

http://codereview.chromium.org/9072011/diff/10001/vm/stub_code_x64.cc#newcode633
vm/stub_code_x64.cc:633: __ leaq(RBX, Address(R10, TIMES_2, fixed_size));  //
R10 is Smi.
It should be TIMES_4.
I'll fix it in my next cl.