Here comes ARM version. Unlike x64 version it has some major differences in
control flow from original ia32 version.

I also found & fixed problem with unbalanced stack in substring stub inherited
from ia32 version. It's rather weird that nothing was crashing.


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
On 2010/03/04 10:20:33, Søren Gjesse wrote:
the comment should actually be "map first string if string check was
performed
above"

Done.

http://codereview.chromium.org/661469/diff/12/1005#newcode9004
src/x64/codegen-x64.cc:9004: // r9: instance type of first string
On 2010/03/04 10:20:33, Søren Gjesse wrote:
Ditto.

Done.

http://codereview.chromium.org/661469/diff/12/1005#newcode9315
src/x64/codegen-x64.cc:9315: __ Move(undefined,
Factory::undefined_value());
On 2010/03/04 09:02:03, Mads Ager wrote:
__ LoadRoot(undefined, Heap::kUndefinedValueRootIndex);

Done.

http://codereview.chromium.org/661469/diff/12/1005#newcode9339
src/x64/codegen-x64.cc:9339: ASSERT_EQ(1, SymbolTableShape::kEntrySize);
On 2010/03/04 10:20:33, Søren Gjesse wrote:
Maybe add

   static int OffsetOfFirstElement()

to SymbolTabel and use it both here and in the ia32 version.

I looked into objects.h and found handy HashTable::kElementsStartOffset
which I decided to use here.

ASSERTion has to be left in place because it has nothing to do with
first element offset calculation --- it checks that  iteration is done
in correct way (correct scale in addrmode)

http://codereview.chromium.org/661469/diff/12/1005#newcode9358
src/x64/codegen-x64.cc:9358: // As we are out of registers save the mask
on the stack and use that
On 2010/03/04 09:02:03, Mads Ager wrote:
Do we have one more register on x64 at the call sites?

As Søren pointed we have kScratchRegister which is not part of calling
convention, but fortunately we do no calls here.

I removed "handmade spilling" and switched to temp = kScratchRegister

http://codereview.chromium.org/661469/diff/12/1005#newcode9361
src/x64/codegen-x64.cc:9361: Register temp = mask;
On 2010/03/04 10:20:33, Søren Gjesse wrote:
I think you can use kScratchRegister as temp here to avoid pushing
mask. This
asumes that JumpIfInstanceTypeIsNotSequentialAscii does not use
kScratchRegister.

Done.

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

Done.


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 ends in scratch3 then it is copied to rax (eax in ia32, r0 in arm)

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(
On 2010/03/04 10:20:33, Søren Gjesse wrote:
Is -> Are (in the ia32 version as well I guess).

Done.

There is no such function in ia32 version. I introduced it here.

http://codereview.chromium.org/661469

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to