LGTM.

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

http://codereview.chromium.org/597022/diff/1/3#newcode4522
src/arm/codegen-arm.cc:4522: int size = JSArray::kSize + elements_size;
Pedantically, you should assert that either of the sizes is a multiple
of kObjectAlignment (otherwise the sum might be less than what is needed
for two aligned objects). A comment should suffice.

http://codereview.chromium.org/597022/diff/1/3#newcode4526
src/arm/codegen-arm.cc:4526: Label slow_case;
Please add a comment stating the order of arguments on the stack, or
make constants for their offsets.

http://codereview.chromium.org/597022/diff/1/3#newcode4548
src/arm/codegen-arm.cc:4548: __ str(r1, FieldMemOperand(r0, i));
There are only seven or eight fields to copy. If you unroll the loop
manually, you can interleave the loads and stores, putting an
instruction between each load and a dependent store.

http://codereview.chromium.org/597022/diff/1/3#newcode4562
src/arm/codegen-arm.cc:4562: __ str(r1, FieldMemOperand(r2, i));
A single unrolling (two loads followed by two stores in each iteration +
potentially a single copy at the end for parity) would reduce the wasted
cycles.

If you have lots of free registers, you can consider using ldrm/strm to
load and store several in few instructions.

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

http://codereview.chromium.org/597022/diff/1/2#newcode6300
src/x64/codegen-x64.cc:6300: for (int i = 0; i < elements_size; i +=
kPointerSize) {
If elements_size gets big (for some value of "big"), it might be better
to make a computed loop in that case.
If that's not expected to happen, this should be fine.

http://codereview.chromium.org/597022

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

Reply via email to