I've just looked at the ARM code so far. I have a few suggestions for
cleanup
(the code is hairy, so I could be reading it wrong).
I'll have a closer look and look at the rest when I get a chance.
http://codereview.chromium.org/7024047/diff/2007/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):
http://codereview.chromium.org/7024047/diff/2007/src/arm/code-stubs-arm.cc#newcode3974
src/arm/code-stubs-arm.cc:3974: // r1 = parameter count (untagged)
(untagged) ==> (tagged)
http://codereview.chromium.org/7024047/diff/2007/src/arm/code-stubs-arm.cc#newcode4011
src/arm/code-stubs-arm.cc:4011: __ cmp(r1, Operand(0));
Since r1 is tagged, can we use Smi::FromInt(0)?
http://codereview.chromium.org/7024047/diff/2007/src/arm/code-stubs-arm.cc#newcode4036
src/arm/code-stubs-arm.cc:4036: __ SmiUntag(r1);
Hmm, if I correctly follow this, the untagged r1 is used in a couple of
compares to 0 and then to compute the length of the parameter map, which
is tagged before storing it in the fixed array.
So I think we could avoid untagging it here, compare to Smi::FromInt(0),
and compute the tagged length directly.
http://codereview.chromium.org/7024047/diff/2007/src/arm/code-stubs-arm.cc#newcode4043
src/arm/code-stubs-arm.cc:4043: // r2 = argument count (untagged)
(untagged) ==> (tagged)
http://codereview.chromium.org/7024047/diff/2007/src/arm/code-stubs-arm.cc#newcode4074
src/arm/code-stubs-arm.cc:4074: __ mov(r6, r0);
This mov might be unnecessary. Can r6 be used instead of r0 below,
preserving the object in r0?
http://codereview.chromium.org/7024047/diff/2007/src/arm/code-stubs-arm.cc#newcode4105
src/arm/code-stubs-arm.cc:4105: __ mov(r3, r4);
Can we avoid this mov by using r3 instead of r4 below:
__ add(r3, r4, Operand(r5, LSL, 2));
__ add(r3, r3, Operand(kParameterMapHeaderSize));
...
http://codereview.chromium.org/7024047/diff/2007/src/arm/code-stubs-arm.cc#newcode4107
src/arm/code-stubs-arm.cc:4107: __ SmiUntag(r5);
Is this untagged and retagged at its use? Maybe my ARM code is rusty,
but I thought:
// No mov(r5, r0) or SmiUntag(r5).
__ add(r4, r4, Operand(r0, LSL, 1));
__ add(r4, r4, Operand(kParameterMapHeaderSize));
http://codereview.chromium.org/7024047/diff/2007/src/arm/code-stubs-arm.cc#newcode4121
src/arm/code-stubs-arm.cc:4121: __ mov(r5, Operand(r0, LSL, 1));
The computation of this value in r5 is duplicated just below. Can we
avoid it with:
__ mov(r5, Operand(r0, LSL, 1));
__ add(r5, r5, Operand(kParameterMapHeaderSize - kHeapObjectTag));
__ str(r1, MemOperand(r3, r5));
__ sub(r5, r5, Operand(kParameterMapHeaderSize -
FixedArray::kHeaderSize));
__ str(r7, MemOperand(r4, r5));
http://codereview.chromium.org/7024047/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
