Port arguments object allocation in generated code to ARM and x64....

Port arguments object allocation in generated code to ARM and x64.

BUG=v8:611
Committed: http://code.google.com/p/v8/source/detail?r=3867

[Søren Thygesen Gjesse, 2010-02-16 08:53:08]

LGTM

http://codereview.chromium.org/605024/diff/1/4
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/605024/diff/1/4#newcode6768
src/arm/codegen-arm.cc:6768: __ ldr(r1, MemOperand(sp, 0 * kPointerSize));
Just 0?

http://codereview.chromium.org/605024/diff/1/4#newcode6774
src/arm/codegen-arm.cc:6774: __ str(r1, MemOperand(sp, 0 * kPointerSize));
Just 0?

http://codereview.chromium.org/605024/diff/1/4#newcode6800
src/arm/codegen-arm.cc:6800: // Copy the JS object part.
Maybe add a comment saying that this includes empty fixed arrays for properties
and elements.

http://codereview.chromium.org/605024/diff/1/4#newcode6809
src/arm/codegen-arm.cc:6809: __ str(r3, FieldMemOperand(r0,
JSObject::kHeaderSize));
Maybe some constants on the object layout would be helpful. That goes for all
platforms.

http://codereview.chromium.org/605024/diff/1/4#newcode6813
src/arm/codegen-arm.cc:6813: __ ldr(r1, MemOperand(sp, 0 * kPointerSize));
Just 0?

http://codereview.chromium.org/605024/diff/1/4#newcode6814
src/arm/codegen-arm.cc:6814: __ str(r1, FieldMemOperand(r0,
JSObject::kHeaderSize + kPointerSize));
Ditto.

http://codereview.chromium.org/605024/diff/1/4#newcode6830
src/arm/codegen-arm.cc:6830: __ str(r3, FieldMemOperand(r4,
FixedArray::kMapOffset));
As FixedArray::kLengthOffset is FixedArray::kMapOffset + kPointerSize you could
use post increment - and have an assert. This could prepare r3 for the loop
below.

http://codereview.chromium.org/605024/diff/1/4#newcode6837
src/arm/codegen-arm.cc:6837: __ str(r3, FieldMemOperand(r4,
FixedArray::kHeaderSize));
I think the add/sub can be replaced by post increment in the ldr/str above, as
r2 and r4 is not used afterwards.

http://codereview.chromium.org/605024/diff/1/3
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/605024/diff/1/3#newcode7383
src/x64/codegen-x64.cc:7383: __ movq(rbx, FieldOperand(rdi, i));
rbx -> kScratchRegister?

http://codereview.chromium.org/605024/diff/1/3#newcode7422
src/x64/codegen-x64.cc:7422: __ testq(rcx, rcx);
Is this testq needed?

[Mads Ager (chromium), 2010-02-16 10:23:51]

http://codereview.chromium.org/605024/diff/1/4
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/605024/diff/1/4#newcode6768
src/arm/codegen-arm.cc:6768: __ ldr(r1, MemOperand(sp, 0 * kPointerSize));
On 2010/02/16 08:53:08, Søren Gjesse wrote:
> Just 0?

Done.

http://codereview.chromium.org/605024/diff/1/4#newcode6774
src/arm/codegen-arm.cc:6774: __ str(r1, MemOperand(sp, 0 * kPointerSize));
On 2010/02/16 08:53:08, Søren Gjesse wrote:
> Just 0?

Done.

http://codereview.chromium.org/605024/diff/1/4#newcode6809
src/arm/codegen-arm.cc:6809: __ str(r3, FieldMemOperand(r0,
JSObject::kHeaderSize));
On 2010/02/16 08:53:08, Søren Gjesse wrote:
> Maybe some constants on the object layout would be helpful. That goes for all
> platforms.

Added a frame-layout comment at the start of the function on all platforms.

http://codereview.chromium.org/605024/diff/1/4#newcode6813
src/arm/codegen-arm.cc:6813: __ ldr(r1, MemOperand(sp, 0 * kPointerSize));
On 2010/02/16 08:53:08, Søren Gjesse wrote:
> Just 0?

Done.

http://codereview.chromium.org/605024/diff/1/4#newcode6814
src/arm/codegen-arm.cc:6814: __ str(r1, FieldMemOperand(r0,
JSObject::kHeaderSize + kPointerSize));
On 2010/02/16 08:53:08, Søren Gjesse wrote:
> Ditto.

Done.

http://codereview.chromium.org/605024/diff/1/4#newcode6830
src/arm/codegen-arm.cc:6830: __ str(r3, FieldMemOperand(r4,
FixedArray::kMapOffset));
On 2010/02/16 08:53:08, Søren Gjesse wrote:
> As FixedArray::kLengthOffset is FixedArray::kMapOffset + kPointerSize you
could
> use post increment - and have an assert. This could prepare r3 for the loop
> below.

I don't understand this comment.  r3 contains the fixed array map which we store
in the newly allocated memory to initialize the elements fixed array.

http://codereview.chromium.org/605024/diff/1/4#newcode6837
src/arm/codegen-arm.cc:6837: __ str(r3, FieldMemOperand(r4,
FixedArray::kHeaderSize));
On 2010/02/16 08:53:08, Søren Gjesse wrote:
> I think the add/sub can be replaced by post increment in the ldr/str above, as
> r2 and r4 is not used afterwards.

Uh, nice.  Done.

http://codereview.chromium.org/605024/diff/1/3
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/605024/diff/1/3#newcode7383
src/x64/codegen-x64.cc:7383: __ movq(rbx, FieldOperand(rdi, i));
On 2010/02/16 08:53:08, Søren Gjesse wrote:
> rbx -> kScratchRegister?

Done.

Also for similar occurrences below.

http://codereview.chromium.org/605024/diff/1/3#newcode7422
src/x64/codegen-x64.cc:7422: __ testq(rcx, rcx);
On 2010/02/16 08:53:08, Søren Gjesse wrote:
> Is this testq needed?

Good catch, no it is not needed as dec sets the zero flag.  Fixed in the ia32
version as well.