Toon, thanks for the review. Sorry for the late response as I was in the Chinese
holiday vacation.

The current usages of Integer32ToSmiField (3 in ic-x64.cc and 2 in
code-stubs-x64.cc) are all dealing with JSArray::Length property. From the
context, this property is less than FixedArray::kMaxLength (128MB) or
FixedDoubleArray::kMaxLength (64MB)when Integer32ToSmiField is used. There is no
overflow check for ia32 port either.

For the 32-bit SMI load/store optimization, I uploaded another CL at
https://codereview.chromium.org/228073004/.

On 2014/04/03 11:47:48, Toon Verwaest wrote:
Mostly LG TM, just one comment.


https://codereview.chromium.org/222133003/diff/20001/src/x64/macro-assembler-x64.cc
File src/x64/macro-assembler-x64.cc (right):


https://codereview.chromium.org/222133003/diff/20001/src/x64/macro-assembler-x64.cc#newcode1112
src/x64/macro-assembler-x64.cc:1112: if (SmiValuesAre32Bits()) {
We shouldn't be able to call Integer32ToSmiField on x32. Int32 obviously
doesn't
fit in a smi, so this can overflow. The optimization of requiring int32 for
storing smi is only available to SmiValuesAre32Bits from within Crankshaft
already, so we should ASSERT that we don't end up in this code-path in the
other
cases.


https://codereview.chromium.org/222133003/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to