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) On 2011/06/07 13:54:10, Kevin Millikin wrote:
(untagged) ==> (tagged)
Done. 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)); On 2011/06/07 13:54:10, Kevin Millikin wrote:
Since r1 is tagged, can we use Smi::FromInt(0)?
Done. http://codereview.chromium.org/7024047/diff/2007/src/arm/code-stubs-arm.cc#newcode4036 src/arm/code-stubs-arm.cc:4036: __ SmiUntag(r1); On 2011/06/07 13:54:10, Kevin Millikin wrote:
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.
Done. 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) On 2011/06/07 13:54:10, Kevin Millikin wrote:
(untagged) ==> (tagged)
Done. http://codereview.chromium.org/7024047/diff/2007/src/arm/code-stubs-arm.cc#newcode4105 src/arm/code-stubs-arm.cc:4105: __ mov(r3, r4); Done. The tricky part is that this code can be jumped over, so I had to introduce a conditional move at line 4079. http://codereview.chromium.org/7024047/diff/2007/src/arm/code-stubs-arm.cc#newcode4107 src/arm/code-stubs-arm.cc:4107: __ SmiUntag(r5); You are right, this is unnecessary. 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)); Done. It makes the code slightly harder to understand, but it should be ok here. http://codereview.chromium.org/7024047/diff/2007/src/x64/code-stubs-x64.cc File src/x64/code-stubs-x64.cc (right): http://codereview.chromium.org/7024047/diff/2007/src/x64/code-stubs-x64.cc#newcode1970 src/x64/code-stubs-x64.cc:1970: // rbx = parameter count (untagged) On 2011/06/07 12:20:19, Lasse Reichstein wrote:
I would move this comment to after the code that ensures the condition
holds. Done. http://codereview.chromium.org/7024047/diff/2007/src/x64/code-stubs-x64.cc#newcode1972 src/x64/code-stubs-x64.cc:1972: __ SmiToInteger64(rbx, rbx); On 2011/06/07 12:20:19, Lasse Reichstein wrote:
There is a SmiToInteger64(Register,Operand) macro that can do this in
one
operation.
Done. http://codereview.chromium.org/7024047/diff/2007/src/x64/code-stubs-x64.cc#newcode2013 src/x64/code-stubs-x64.cc:2013: __ testq(rbx, rbx); I will keep the current code. http://codereview.chromium.org/7024047/diff/2007/src/x64/code-stubs-x64.cc#newcode2037 src/x64/code-stubs-x64.cc:2037: __ SmiToInteger64(rbx, rbx); On 2011/06/07 12:20:19, Lasse Reichstein wrote:
And another 64-bit load.
Done. http://codereview.chromium.org/7024047/diff/2007/src/x64/code-stubs-x64.cc#newcode2073 src/x64/code-stubs-x64.cc:2073: __ Integer64PlusConstantToSmi(rcx, rcx, 0); On 2011/06/07 12:20:19, Lasse Reichstein wrote:
Just use Integer32ToSmi when the costant is zero. The macro will
truncate the
value to a signed 32-bit value after addition anyway. It probably should check for the constant being zero. It really is a stupid macro which doesn't provide any good
optimizations with
the current smi implementation.
Done. http://codereview.chromium.org/7024047/diff/2007/src/x64/code-stubs-x64.cc#newcode2154 src/x64/code-stubs-x64.cc:2154: __ testq(rax, rax); On 2011/06/07 12:20:19, Lasse Reichstein wrote:
If rax is a smi, use SmiTest(rax); for readability. It'll do the same thing :)
Done. http://codereview.chromium.org/7024047/diff/2007/src/x64/code-stubs-x64.cc#newcode2205 src/x64/code-stubs-x64.cc:2205: __ Integer64PlusConstantToSmi(rcx, rcx, 0); On 2011/06/07 12:20:19, Lasse Reichstein wrote:
Integer32ToSmi.
Done. http://codereview.chromium.org/7024047/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
