Still LGTM, but feel free to wait for Vitlay's review as well.
http://codereview.chromium.org/7744052/diff/1/src/ia32/code-stubs-ia32.cc File src/ia32/code-stubs-ia32.cc (right): http://codereview.chromium.org/7744052/diff/1/src/ia32/code-stubs-ia32.cc#newcode5728 src/ia32/code-stubs-ia32.cc:5728: const uint32_t kSlicedNotConsMask = kSlicedStringTag & ~kConsStringTag; On 2011/08/29 15:17:07, Yang wrote:
On 2011/08/29 14:51:27, antonm wrote: > On 2011/08/26 16:10:55, antonm wrote: > > lift those constants into objects.h? > > any response?
I did move these two lines into objects.h Maybe you were looking in the wrong diff.
Sorry, didn't notice that. Usually people respond w/ something like Done. http://codereview.chromium.org/7744052/diff/1/src/ia32/code-stubs-ia32.cc#newcode5747 src/ia32/code-stubs-ia32.cc:5747: __ mov(edi, eax); On 2011/08/29 15:17:07, Yang wrote:
On 2011/08/29 14:51:27, antonm wrote: > On 2011/08/29 12:55:56, Yang wrote: > > On 2011/08/26 16:10:55, antonm wrote: > > > maybe keep the parent string in eax and save this move? > > > > eax has to be used as destination when allocation the new sliced
string and
as > > return value. > > I am not sure, apparently you can allocate into any register you
like, at
least > w/ some minimal amount of work (at least AllocateInNewSpace behaves
this way)
Yes but the issue is that the return value of this code stub is fixed
to be eax.
Now I could of course load the original string into edi to begin with,
but then
I would have to move edi into eax in case the substring is the same as
the
original string. So the issue is that the return value has to be eax, the original
string has to
be loaded into eax as well since we need to be able to return the
original
string immediately if necessary.
ok http://codereview.chromium.org/7744052/diff/9001/src/ia32/code-stubs-ia32.cc File src/ia32/code-stubs-ia32.cc (right): http://codereview.chromium.org/7744052/diff/9001/src/ia32/code-stubs-ia32.cc#newcode5725 src/ia32/code-stubs-ia32.cc:5725: STATIC_ASSERT(kIsIndirectStringMask == (kSlicedStringTag & kConsStringTag)); may those go too? http://codereview.chromium.org/7744052/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
