I also simplified the code in stub-cache.cc which constructed the name of the code stub, and removed the changes to string-stream.{h,cc} in favor of adding
two symbols to heap.h.


http://codereview.chromium.org/6315004/diff/1/src/ia32/stub-cache-ia32.cc
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/6315004/diff/1/src/ia32/stub-cache-ia32.cc#newcode3574
src/ia32/stub-cache-ia32.cc:3574: __ pop(ecx);
On 2011/01/14 14:58:17, William Hesse wrote:
Why are SSE3 features used on ia32, but not on x64?  I see that the
int32 case
uses cvttsd2siq on x64, but on the other cases, if cvttsd2si is best
on x64, why
isn't it best on ia32.

x64 can assume SSE2. This plus the availability of cvttsd2siq means x64
doesn't need to touch the FP stack at all. The 32-bit code must use the
FP stack at least in some cases. It would make the 32-bit code much more
complicated to use the SSE2 instructions as the common case and only use
fistpp / fistp as the fallback for all of the array types.

http://codereview.chromium.org/6315004/diff/1/src/ia32/stub-cache-ia32.cc#newcode3650
src/ia32/stub-cache-ia32.cc:3650: Label not_infinity;
On 2011/01/14 14:58:17, William Hesse wrote:
I don't see why numbers outside the 32-bit int range are not returning
MIN_INT,
which is then not screened out by this test.  So finite large numbers
will
return MIN_INT, but on x64, they will return MIN_INT64 or large
numbers mod
2^32.  Isn't this a difference between platforms?  What does the spec
say.

You're right, and the x64 code's behavior is what is desired here. I've
changed to use the fisttp_d / fistp_d code path for both the signed and
unsigned int array cases; this allows the +/-Infinity test to be
removed, just like in the x64 code.

http://codereview.chromium.org/6315004/diff/1/src/x64/assembler-x64.cc
File src/x64/assembler-x64.cc (right):

http://codereview.chromium.org/6315004/diff/1/src/x64/assembler-x64.cc#newcode2723
src/x64/assembler-x64.cc:2723:
On 2011/01/14 14:58:17, William Hesse wrote:
disasm-x64.cc has an assert on line 1117 that will now fail.  It needs
to be
changed, to look like the way disasm-x6 handles cvttsd2si.

Done. Tested in 64-bit Chromium with --js-flags="--print_code_stubs" and
verified no assertion failures with the typed array unit tests.

http://codereview.chromium.org/6315004/

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

Reply via email to