Driveby comments on X64 code (style only, haven't checked what it does).
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) I would move this comment to after the code that ensures the condition holds. http://codereview.chromium.org/7024047/diff/2007/src/x64/code-stubs-x64.cc#newcode1972 src/x64/code-stubs-x64.cc:1972: __ SmiToInteger64(rbx, rbx); There is a SmiToInteger64(Register,Operand) macro that can do this in one operation. http://codereview.chromium.org/7024047/diff/2007/src/x64/code-stubs-x64.cc#newcode1989 src/x64/code-stubs-x64.cc:1989: __ SmiToInteger64(rcx, rcx); Ditto here. http://codereview.chromium.org/7024047/diff/2007/src/x64/code-stubs-x64.cc#newcode2013 src/x64/code-stubs-x64.cc:2013: __ testq(rbx, rbx); You could get away with using just testl, since rbx is known to be a sign-extended 32-bit number. However, that would require a comment that's probably more bother than the extra byte of 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); And another 64-bit load. 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); 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. http://codereview.chromium.org/7024047/diff/2007/src/x64/code-stubs-x64.cc#newcode2154 src/x64/code-stubs-x64.cc:2154: __ testq(rax, rax); If rax is a smi, use SmiTest(rax); for readability. It'll do the same thing :) 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); Integer32ToSmi. http://codereview.chromium.org/7024047/diff/2007/src/x64/code-stubs-x64.cc#newcode2315 src/x64/code-stubs-x64.cc:2315: __ SmiToInteger64(rcx, rcx); It seems rcx could be just a 32-bit value here. It's only used as a counter, not as a part of an address calculation. http://codereview.chromium.org/7024047/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
