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? http://codereview.chromium.org/507036 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
