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; On 2010/02/10 17:41:27, Lasse Reichstein wrote:
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.
Added a comment stating that all sizes here are multiples of kPointerSize. http://codereview.chromium.org/597022/diff/1/3#newcode4526 src/arm/codegen-arm.cc:4526: Label slow_case; On 2010/02/10 17:41:27, Lasse Reichstein wrote:
Please add a comment stating the order of arguments on the stack, or
make
constants for their offsets.
Done. http://codereview.chromium.org/597022/diff/1/3#newcode4548 src/arm/codegen-arm.cc:4548: __ str(r1, FieldMemOperand(r0, i)); On 2010/02/10 17:41:27, Lasse Reichstein wrote:
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.
Good point. Let's experiment with that on all platforms as a separate change. http://codereview.chromium.org/597022/diff/1/3#newcode4562 src/arm/codegen-arm.cc:4562: __ str(r1, FieldMemOperand(r2, i)); On 2010/02/10 17:41:27, Lasse Reichstein wrote:
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.
Good point. Let's experiment with that on all platforms as a separate change. 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) { On 2010/02/10 17:41:27, Lasse Reichstein wrote:
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.
Elements size is bounded because we check for FastCloneShallowArrayStub::kMaximumLength before calling the stub. http://codereview.chromium.org/597022 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
