Use one runtime call for creating object/array literals in...

Use one runtime call for creating object/array literals in
the code generator.

The runtime function checks if it needs to create a boilerplate
object or if it can clone from an existing boilerplate.

This is already done in the top-level compiler. 


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

[fschneider, 2009-12-17 13:03:27]

Ported this change from the fast compiler also to the normal compiler.

[fschneider, 2009-12-17 19:18:44]

Uploaded new version. Added Kasper to review.

Changes: 
Merged with Kasper's fast array literal allocation stub for IA-32.

This is a working version. We have the check for the boilerplate back in the
generated code for the fast case. We may want to move this into the stub
somehow. Net effect now is 1.5% less code size.

[fschneider, 2009-12-21 13:14:04]

I'm moving the check if the boilerplate exists into the fast clone stub.

Still working on this (not ready for review yet).

[fschneider, 2009-12-22 10:42:04]

Ready for review now.

The FastCloneStub (only on IA-32) take now 3 arguments and the check for the
boilerplate object is performed in the stub and the deferred code is not needed
anymore for array+object literals.

[Kevin Millikin (Chromium), 2009-12-22 11:58:14]

I have some suggestions for cleanup, but otherwise it LGTM.

http://codereview.chromium.org/507036/diff/6/9
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/507036/diff/6/9#newcode2690
src/arm/codegen-arm.cc:2690: __ stm(db_w, sp, r2.bit() | r1.bit() | r0.bit());
You might add a function to the ARM virtual frame to do this.  It seems safer if
we ever implement the virtual frame on ARM.

void VirtualFrame::EmitStoreMultiple(int count, RegList srcs) {
  ASSERT(stack_pointer_ == element_count() - 1);
  Adjust(count);
  __ stm(db_w, sp, srcs);
}

http://codereview.chromium.org/507036/diff/6/7
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/507036/diff/6/7#newcode4371
src/ia32/codegen-ia32.cc:4371: if (node->depth() == 1 &&
It seems simpler to factor out all the shared code between these branches:

frame_->Push(&literals);
frame_->Push(Smi::FromInt(node->literal_index());
frame_->Push(node->literals());
Result clone;
if (node->depth() > 1) {
  clone = frame_->CallRuntime(Runtime::kCreateArrayLiteral, 3);
} else if (length > FastCloneShallowArrayStub::kMaximumLenght) {
  clone = frame_->CallRuntime(Runtime::kCreateArrayLiteralShallow, 3);
} else {
  FastCloneShallowArrayStub stub(length);
  clone = frame_->CallStub(&stub, 3);
}
frame_->Push(&clone);

http://codereview.chromium.org/507036/diff/6/7#newcode4378
src/ia32/codegen-ia32.cc:4378: frame_->Push(node->literals());
The name ArrayLiteral::literals() is unfortunate.  It's not the literals array
of the function and the elements do not have type Literal*.  Maybe something
like 'constant_elements' or 'compile_time_elements' is clearer.

http://codereview.chromium.org/507036/diff/6/7#newcode6641
src/ia32/codegen-ia32.cc:6641: __ shr(eax, kSmiTagSize);
You don't need to untag the index:

ASSERT(kPointerSize == 4);
__ mov(ecx, FieldOperand(ecx, eax, times_2,
                         FixedArray::kHeaderSize));
__ cmp(ecx, Factor::undefined_value());
...

http://codereview.chromium.org/507036/diff/6/7#newcode6652
src/ia32/codegen-ia32.cc:6652: // allocation. This avoid multiple limit checks.
"avoid" ==> "avoids"

http://codereview.chromium.org/507036/diff/6/7#newcode6681
src/ia32/codegen-ia32.cc:6681: ExternalReference
runtime2(Runtime::kCreateArrayLiteralShallow);
runtime2?

[fschneider, 2009-12-22 12:40:09]

http://codereview.chromium.org/507036/diff/6/9
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/507036/diff/6/9#newcode2690
src/arm/codegen-arm.cc:2690: __ stm(db_w, sp, r2.bit() | r1.bit() | r0.bit());
On 2009/12/22 11:58:14, Kevin Millikin wrote:
> You might add a function to the ARM virtual frame to do this.  It seems safer
if
> we ever implement the virtual frame on ARM.
> 
> void VirtualFrame::EmitStoreMultiple(int count, RegList srcs) {
>   ASSERT(stack_pointer_ == element_count() - 1);
>   Adjust(count);
>   __ stm(db_w, sp, srcs);
> }
>   

Done. Good idea, otherwise too easy to forget calling frame_->Adjust together
with the __ stm instruction.

http://codereview.chromium.org/507036/diff/6/7
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/507036/diff/6/7#newcode4371
src/ia32/codegen-ia32.cc:4371: if (node->depth() == 1 &&
On 2009/12/22 11:58:14, Kevin Millikin wrote:
> It seems simpler to factor out all the shared code between these branches:
> 
> frame_->Push(&literals);
> frame_->Push(Smi::FromInt(node->literal_index());
> frame_->Push(node->literals());
> Result clone;
> if (node->depth() > 1) {
>   clone = frame_->CallRuntime(Runtime::kCreateArrayLiteral, 3);
> } else if (length > FastCloneShallowArrayStub::kMaximumLenght) {
>   clone = frame_->CallRuntime(Runtime::kCreateArrayLiteralShallow, 3);
> } else {
>   FastCloneShallowArrayStub stub(length);
>   clone = frame_->CallStub(&stub, 3);
> }
> frame_->Push(&clone);

Done.

http://codereview.chromium.org/507036/diff/6/7#newcode4378
src/ia32/codegen-ia32.cc:4378: frame_->Push(node->literals());
On 2009/12/22 11:58:14, Kevin Millikin wrote:
> The name ArrayLiteral::literals() is unfortunate.  It's not the literals array
> of the function and the elements do not have type Literal*.  Maybe something
> like 'constant_elements' or 'compile_time_elements' is clearer.

Done.

http://codereview.chromium.org/507036/diff/6/7#newcode6641
src/ia32/codegen-ia32.cc:6641: __ shr(eax, kSmiTagSize);
On 2009/12/22 11:58:14, Kevin Millikin wrote:
> You don't need to untag the index:
> 
> ASSERT(kPointerSize == 4);
> __ mov(ecx, FieldOperand(ecx, eax, times_2,
>                          FixedArray::kHeaderSize));
> __ cmp(ecx, Factor::undefined_value());
> ...

Done.

http://codereview.chromium.org/507036/diff/6/7#newcode6652
src/ia32/codegen-ia32.cc:6652: // allocation. This avoid multiple limit checks.
On 2009/12/22 11:58:14, Kevin Millikin wrote:
> "avoid" ==> "avoids"

Done.

http://codereview.chromium.org/507036/diff/6/7#newcode6681
src/ia32/codegen-ia32.cc:6681: ExternalReference
runtime2(Runtime::kCreateArrayLiteralShallow);
On 2009/12/22 11:58:14, Kevin Millikin wrote:
> runtime2?

Oops. Done.