Second step in allocating objects in generated code on ARM

Second step in allocating objects in generated code on ARM.

Objects which require an additional fixed array to be allocated now have this allocated in generated code as well. Added allocation flags to the macro assembler new space allocation routines.

Changed the ia32 and x64 macro assemblers to take allocation flags to the allocation routines instead of boolean flag.

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

[Erik Corry, 2009-09-07 07:53:52]

LGTM

http://codereview.chromium.org/201015/diff/1/13
File src/arm/builtins-arm.cc (right):

http://codereview.chromium.org/201015/diff/1/13#newcode1
Line 1: // Copyright 2006-2008 the V8 project authors. All rights reserved.
2009

http://codereview.chromium.org/201015/diff/1/13#newcode139
Line 139: ASSERT(Heap::MaxObjectSizeInPagedSpace() >=
JSObject::kMaxInstanceSize);
Assert not necessary since young space can contain objects that are too large
for paged space.

http://codereview.chromium.org/201015/diff/1/13#newcode211
Line 211: (FixedArray::kHeaderSize + 255 * kPointerSize));
Assert not necessary for same reason.

http://codereview.chromium.org/201015/diff/1/13#newcode213
Line 213: __ add(r0, r0, Operand(r3));
This mov and add can be one instruction.

http://codereview.chromium.org/201015/diff/1/15
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/201015/diff/1/15#newcode785
Line 785: if (!(flags & RESULT_CONTAINS_TOP)) {
We should use comparisons against zero instead of implicit integer-boolean
conversions.

http://codereview.chromium.org/201015/diff/1/15#newcode788
Line 788: #ifdef DEBUG
I think it makes debugging easier if we generate the same code in debug and
release modes.  I realise that we don't do that consistently at the moment.

http://codereview.chromium.org/201015/diff/1/15#newcode809
Line 809: if (flags & TAG_OBJECT) {
Implicit bool conversion.

http://codereview.chromium.org/201015/diff/1/15#newcode860
Line 860: if (flags & TAG_OBJECT) {
And here.

http://codereview.chromium.org/201015/diff/1/12
File src/arm/macro-assembler-arm.h (right):

http://codereview.chromium.org/201015/diff/1/12#newcode70
Line 70: enum AllocationFalgs {
Falgs -> Flags

http://codereview.chromium.org/201015/diff/1/12#newcode71
Line 71: NO_ALLOCATION_FLAGS = 0,
A one-line comment as to what these mean would be nice.

http://codereview.chromium.org/201015/diff/1/3
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/201015/diff/1/3#newcode745
Line 745: if (flags & TAG_OBJECT) {
Implicit conversions in this file too.

http://codereview.chromium.org/201015/diff/1/5
File src/ia32/macro-assembler-ia32.h (right):

http://codereview.chromium.org/201015/diff/1/5#newcode59
Line 59: enum AllocationFlags {
Can we move this up to assembler.h?

[Søren Thygesen Gjesse, 2009-09-07 09:44:12]

http://codereview.chromium.org/201015/diff/1/13
File src/arm/builtins-arm.cc (right):

http://codereview.chromium.org/201015/diff/1/13#newcode1
Line 1: // Copyright 2006-2008 the V8 project authors. All rights reserved.
On 2009/09/07 07:53:52, Erik Corry wrote:
> 2009

Done.

http://codereview.chromium.org/201015/diff/1/13#newcode139
Line 139: ASSERT(Heap::MaxObjectSizeInPagedSpace() >=
JSObject::kMaxInstanceSize);
On 2009/09/07 07:53:52, Erik Corry wrote:
> Assert not necessary since young space can contain objects that are too large
> for paged space.

Done.

http://codereview.chromium.org/201015/diff/1/13#newcode211
Line 211: (FixedArray::kHeaderSize + 255 * kPointerSize));
On 2009/09/07 07:53:52, Erik Corry wrote:
> Assert not necessary for same reason.

Done.

http://codereview.chromium.org/201015/diff/1/13#newcode213
Line 213: __ add(r0, r0, Operand(r3));
On 2009/09/07 07:53:52, Erik Corry wrote:
> This mov and add can be one instruction.

Done.

http://codereview.chromium.org/201015/diff/1/15
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/201015/diff/1/15#newcode785
Line 785: if (!(flags & RESULT_CONTAINS_TOP)) {
On 2009/09/07 07:53:52, Erik Corry wrote:
> We should use comparisons against zero instead of implicit integer-boolean
> conversions.

Done.

http://codereview.chromium.org/201015/diff/1/15#newcode788
Line 788: #ifdef DEBUG
On 2009/09/07 07:53:52, Erik Corry wrote:
> I think it makes debugging easier if we generate the same code in debug and
> release modes.  I realise that we don't do that consistently at the moment.

This is an assert which needs some additional code to check that result actually
contains allocation top on entry. Any use of MacroAssembler::Assert will cause
code which is not the same in release and debug mode. scratch2 is used again
just after this, so this side effect will not cause different behavior with
respect to register content between release and debug.

Added a comment to this #ifdef/#endif block.

http://codereview.chromium.org/201015/diff/1/15#newcode809
Line 809: if (flags & TAG_OBJECT) {
On 2009/09/07 07:53:52, Erik Corry wrote:
> Implicit bool conversion.

Done.

http://codereview.chromium.org/201015/diff/1/15#newcode860
Line 860: if (flags & TAG_OBJECT) {
On 2009/09/07 07:53:52, Erik Corry wrote:
> And here.

Done.

http://codereview.chromium.org/201015/diff/1/12
File src/arm/macro-assembler-arm.h (right):

http://codereview.chromium.org/201015/diff/1/12#newcode70
Line 70: enum AllocationFalgs {
On 2009/09/07 07:53:52, Erik Corry wrote:
> Falgs -> Flags

Done.

http://codereview.chromium.org/201015/diff/1/12#newcode71
Line 71: NO_ALLOCATION_FLAGS = 0,
On 2009/09/07 07:53:52, Erik Corry wrote:
> A one-line comment as to what these mean would be nice.

Done.

http://codereview.chromium.org/201015/diff/1/3
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/201015/diff/1/3#newcode745
Line 745: if (flags & TAG_OBJECT) {
On 2009/09/07 07:53:52, Erik Corry wrote:
> Implicit conversions in this file too.

Done here and 3 other places.

http://codereview.chromium.org/201015/diff/1/5
File src/ia32/macro-assembler-ia32.h (right):

http://codereview.chromium.org/201015/diff/1/5#newcode59
Line 59: enum AllocationFlags {
On 2009/09/07 07:53:52, Erik Corry wrote:
> Can we move this up to assembler.h?

It should be moved to macro-assembler.h then and together with the other enums
which are used on all platforms. I will suggest that in a separate changelist.