Optimize KeyedStoreGeneric for Smi arrays.

Optimize KeyedStoreGeneric for Smi arrays.

BUG=
TEST=

Committed: http://code.google.com/p/v8/source/detail?r=9456

[danno, 2011-09-26 14:08:25]

PTAL

[Yang, 2011-09-26 16:03:46]

Came across this when porting.

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

http://codereview.chromium.org/8022002/diff/6003/src/ia32/stub-cache-ia32.cc#...
src/ia32/stub-cache-ia32.cc:3980: Label miss_force_generic, smi_value, is_nan,
maybe_nan;
Get rid of unused labels.

[Rico, 2011-09-27 12:57:53]

LGTM

http://codereview.chromium.org/8022002/diff/13001/src/ia32/ic-ia32.cc
File src/ia32/ic-ia32.cc (right):

http://codereview.chromium.org/8022002/diff/13001/src/ia32/ic-ia32.cc#newcode739
src/ia32/ic-ia32.cc:739: Label check_extra_double, array, extra;
check_extra_double -> check_is_fast_double_array?

http://codereview.chromium.org/8022002/diff/13001/src/ia32/ic-ia32.cc#newcode788
src/ia32/ic-ia32.cc:788: // Add 1 to receiver->length, and go to fast array
write.
Update comment

http://codereview.chromium.org/8022002/diff/13001/src/ia32/ic-ia32.cc#newcode801
src/ia32/ic-ia32.cc:801: // Array case: Get the length and the elements array
from the JS
This comment seems outdated now, please update to reflect what we do now

http://codereview.chromium.org/8022002/diff/13001/src/ia32/ic-ia32.cc#newcode815
src/ia32/ic-ia32.cc:815: // Fast case: Do the store.
This comment is also slightly misleading now, since we will not necessarily
actuallty do the store here, we might jump to fast_double case and through there
even go to runtime

http://codereview.chromium.org/8022002/diff/13001/src/ia32/ic-ia32.cc#newcode834
src/ia32/ic-ia32.cc:834: __ CheckFastObjectElements(edi, &slow, Label::kNear);
Could we add something like the following to (all) the registrer overviews:
// edi: receiver map
this is loaded way back.
Also, maybe add comment saying why we will not bailout here if we have a fast
double array (we already jumped over this if we did)

http://codereview.chromium.org/8022002/diff/13001/src/ia32/macro-assembler-ia...
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/8022002/diff/13001/src/ia32/macro-assembler-ia...
src/ia32/macro-assembler-ia32.cc:439: scratch2);
indention

http://codereview.chromium.org/8022002/diff/13001/src/ia32/macro-assembler-ia...
src/ia32/macro-assembler-ia32.cc:463: // Value is a smi. convert to a double and
store.
Capital c after period

http://codereview.chromium.org/8022002/diff/13001/src/ia32/macro-assembler-ia...
src/ia32/macro-assembler-ia32.cc:468: fild_s(Operand(esp, 0));
Why not also use SSE2 instructions here if available, cvtsi2sd on scratch1 (no
need to push) and then store the loaded xmm register to the elements array.
Don't know it this makes a performance difference though

[danno, 2011-09-27 16:14:57]

Feedback addressed, landing

http://codereview.chromium.org/8022002/diff/13001/src/ia32/ic-ia32.cc
File src/ia32/ic-ia32.cc (right):

http://codereview.chromium.org/8022002/diff/13001/src/ia32/ic-ia32.cc#newcode739
src/ia32/ic-ia32.cc:739: Label check_extra_double, array, extra;
On 2011/09/27 12:57:53, Rico wrote:
> check_extra_double -> check_is_fast_double_array?

Done.

http://codereview.chromium.org/8022002/diff/13001/src/ia32/ic-ia32.cc#newcode788
src/ia32/ic-ia32.cc:788: // Add 1 to receiver->length, and go to fast array
write.
On 2011/09/27 12:57:53, Rico wrote:
> Update comment

Done.

http://codereview.chromium.org/8022002/diff/13001/src/ia32/ic-ia32.cc#newcode801
src/ia32/ic-ia32.cc:801: // Array case: Get the length and the elements array
from the JS
On 2011/09/27 12:57:53, Rico wrote:
> This comment seems outdated now, please update to reflect what we do now

Done.

http://codereview.chromium.org/8022002/diff/13001/src/ia32/ic-ia32.cc#newcode815
src/ia32/ic-ia32.cc:815: // Fast case: Do the store.
On 2011/09/27 12:57:53, Rico wrote:
> This comment is also slightly misleading now, since we will not necessarily
> actuallty do the store here, we might jump to fast_double case and through
there
> even go to runtime 

Done.

http://codereview.chromium.org/8022002/diff/13001/src/ia32/ic-ia32.cc#newcode834
src/ia32/ic-ia32.cc:834: __ CheckFastObjectElements(edi, &slow, Label::kNear);
On 2011/09/27 12:57:53, Rico wrote:
> Could we add something like the following to (all) the registrer overviews:
> // edi: receiver map
> this is loaded way back.
> Also, maybe add comment saying why we will not bailout here if we have a fast
> double array (we already jumped over this if we did)

Done.

http://codereview.chromium.org/8022002/diff/13001/src/ia32/macro-assembler-ia...
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/8022002/diff/13001/src/ia32/macro-assembler-ia...
src/ia32/macro-assembler-ia32.cc:439: scratch2);
On 2011/09/27 12:57:53, Rico wrote:
> indention

Done.

http://codereview.chromium.org/8022002/diff/13001/src/ia32/macro-assembler-ia...
src/ia32/macro-assembler-ia32.cc:463: // Value is a smi. convert to a double and
store.
On 2011/09/27 12:57:53, Rico wrote:
> Capital c after period

Done.

http://codereview.chromium.org/8022002/diff/13001/src/ia32/macro-assembler-ia...
src/ia32/macro-assembler-ia32.cc:468: fild_s(Operand(esp, 0));
On 2011/09/27 12:57:53, Rico wrote:
> Why not also use SSE2 instructions here if available, cvtsi2sd on scratch1 (no
> need to push) and then store the loaded xmm register to the elements array.
> Don't know it this makes a performance difference though

Done.