LGTM
http://codereview.chromium.org/661469/diff/12/1005 File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/661469/diff/12/1005#newcode9003 src/x64/codegen-x64.cc:9003: // r8: instance type of first string the comment should actually be "map first string if string check was performed above" http://codereview.chromium.org/661469/diff/12/1005#newcode9004 src/x64/codegen-x64.cc:9004: // r9: instance type of first string Ditto. http://codereview.chromium.org/661469/diff/12/1005#newcode9339 src/x64/codegen-x64.cc:9339: ASSERT_EQ(1, SymbolTableShape::kEntrySize); Maybe add static int OffsetOfFirstElement() to SymbolTabel and use it both here and in the ia32 version. http://codereview.chromium.org/661469/diff/12/1005#newcode9361 src/x64/codegen-x64.cc:9361: Register temp = mask; I think you can use kScratchRegister as temp here to avoid pushing mask. This asumes that JumpIfInstanceTypeIsNotSequentialAscii does not use kScratchRegister. http://codereview.chromium.org/661469/diff/12/1006 File src/x64/codegen-x64.h (right): http://codereview.chromium.org/661469/diff/12/1006#newcode781 src/x64/codegen-x64.h:781: // string is found the code falls through with the string in register eax. On 2010/03/04 09:02:03, Mads Ager wrote:
eax -> rax
But really, I think this comment should be changed on ia32 as well. I
guess the
string will be left in one of the argument registers (c1, c2?) which
does not
have to be eax/rax.
It actually does fall through with result in explicitly eax on ia32 so that there can be a ret instruction just after. http://codereview.chromium.org/661469/diff/12/1004 File src/x64/macro-assembler-x64.h (right): http://codereview.chromium.org/661469/diff/12/1004#newcode436 src/x64/macro-assembler-x64.h:436: void JumpIfBothInstanceTypesIsNotSequentialAscii( Is -> Are (in the ia32 version as well I guess). http://codereview.chromium.org/661469 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
