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.

http://codereview.chromium.org/507036

-- 
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to