Support Smi->Double->HeapObject transitions in constructed Arrays.

Support Smi->Double->HeapObject transitions in constructed Arrays.

Also several bugs with Smi/double elements handling and make Ensure* routines more flexible. 

BUG=none
TEST=test/mjsunit/array-construct-transition.js

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

[danno, 2011-12-07 12:57:27]

PTAL:

Reviewer guidance:

* The main change is to support element type transitions in the Array()
constructor. The platform specific builtin to construct Arrays now bails out to
the C++ runtime code when a transition is necessary, and the C++ runtime code
support the transition.

* There were tweaks made to the EnsureCanContain* functions to more precisely
support the different contexts that elements transitions are needed in.  These
are needed because the Array code need the Ensure call to generate a
FAST_DOUBLE_ARRAY ElementsKind, which was previously not supported.

[Jakob Kummerow, 2011-12-07 17:02:43]

http://codereview.chromium.org/8820014/diff/4001/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):

http://codereview.chromium.org/8820014/diff/4001/src/arm/builtins-arm.cc#newc...
src/arm/builtins-arm.cc:397: Label bailout;
Unused label. (Does this even compile with -Werror?)

http://codereview.chromium.org/8820014/diff/4001/src/arm/builtins-arm.cc#newc...
src/arm/builtins-arm.cc:398: __ mov(r7, sp);
Why? I don't see how sp would get overwritten.

http://codereview.chromium.org/8820014/diff/4001/src/builtins.cc
File src/builtins.cc (right):

http://codereview.chromium.org/8820014/diff/4001/src/builtins.cc#newcode238
src/builtins.cc:238: array->set_length(Smi::FromInt(0));
A comment explaining why the length must be set to 0 first would be nice.

http://codereview.chromium.org/8820014/diff/4001/src/builtins.cc#newcode255
src/builtins.cc:255: FixedArrayBase* elms = FixedArrayBase::cast(obj);
Thanks to templates, you can clean this up a little by writing:
if (!maybe_obj->To(&elms)) return maybe_obj;
Then you wouldn't need "Object* obj;" at all. I don't mind either way, your
call.

http://codereview.chromium.org/8820014/diff/4001/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):

http://codereview.chromium.org/8820014/diff/4001/src/ia32/builtins-ia32.cc#ne...
src/ia32/builtins-ia32.cc:1246: // esp[4] : argc
nit: excess space

http://codereview.chromium.org/8820014/diff/4001/src/ia32/builtins-ia32.cc#ne...
src/ia32/builtins-ia32.cc:1265: // esp[4] : argc
same nit here

http://codereview.chromium.org/8820014/diff/4001/src/ia32/builtins-ia32.cc#ne...
src/ia32/builtins-ia32.cc:1286: // esp[4] : argc
same nit here

http://codereview.chromium.org/8820014/diff/4001/src/objects-inl.h
File src/objects-inl.h (right):

http://codereview.chromium.org/8820014/diff/4001/src/objects-inl.h#newcode1282
src/objects-inl.h:1282: mode);
nit: no need for line break

http://codereview.chromium.org/8820014/diff/4001/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/8820014/diff/4001/src/objects.cc#newcode8370
src/objects.cc:8370: void JSArray::Expand(int required_size) {
Instead of all this code here, we should be able to make use of
SetFastElementsCapacityAndLength (passing in the original length, and
required_size as capacity).

http://codereview.chromium.org/8820014/diff/4001/test/mjsunit/array-construct...
File test/mjsunit/array-construct-transition.js (right):

http://codereview.chromium.org/8820014/diff/4001/test/mjsunit/array-construct...
test/mjsunit/array-construct-transition.js:33: a = new Array(0, 1, 2);
nit: "var a", please. Same for b and c.

[danno, 2011-12-08 15:09:09]

Please take another look

http://codereview.chromium.org/8820014/diff/4001/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):

http://codereview.chromium.org/8820014/diff/4001/src/arm/builtins-arm.cc#newc...
src/arm/builtins-arm.cc:397: Label bailout;
On 2011/12/07 17:02:47, Jakob wrote:
> Unused label. (Does this even compile with -Werror?)

Done.

http://codereview.chromium.org/8820014/diff/4001/src/arm/builtins-arm.cc#newc...
src/arm/builtins-arm.cc:398: __ mov(r7, sp);
I didn't see it initially, either, and it cost me about an hour. MemOperand(r7,
kPointerSize, PostIndex) below changes r7. 
On 2011/12/07 17:02:47, Jakob wrote:
> Why? I don't see how sp would get overwritten.

http://codereview.chromium.org/8820014/diff/4001/src/builtins.cc
File src/builtins.cc (right):

http://codereview.chromium.org/8820014/diff/4001/src/builtins.cc#newcode238
src/builtins.cc:238: array->set_length(Smi::FromInt(0));
It's actually unneeded. Removed.
On 2011/12/07 17:02:47, Jakob wrote:
> A comment explaining why the length must be set to 0 first would be nice.

http://codereview.chromium.org/8820014/diff/4001/src/builtins.cc#newcode255
src/builtins.cc:255: FixedArrayBase* elms = FixedArrayBase::cast(obj);
On 2011/12/07 17:02:47, Jakob wrote:
> Thanks to templates, you can clean this up a little by writing:
> if (!maybe_obj->To(&elms)) return maybe_obj;
> Then you wouldn't need "Object* obj;" at all. I don't mind either way, your
> call.

Done.

http://codereview.chromium.org/8820014/diff/4001/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):

http://codereview.chromium.org/8820014/diff/4001/src/ia32/builtins-ia32.cc#ne...
src/ia32/builtins-ia32.cc:1246: // esp[4] : argc
On 2011/12/07 17:02:47, Jakob wrote:
> nit: excess space

Done.

http://codereview.chromium.org/8820014/diff/4001/src/ia32/builtins-ia32.cc#ne...
src/ia32/builtins-ia32.cc:1265: // esp[4] : argc
On 2011/12/07 17:02:47, Jakob wrote:
> same nit here

Done.

http://codereview.chromium.org/8820014/diff/4001/src/ia32/builtins-ia32.cc#ne...
src/ia32/builtins-ia32.cc:1286: // esp[4] : argc
On 2011/12/07 17:02:47, Jakob wrote:
> same nit here

Done.

http://codereview.chromium.org/8820014/diff/4001/src/objects-inl.h
File src/objects-inl.h (right):

http://codereview.chromium.org/8820014/diff/4001/src/objects-inl.h#newcode1282
src/objects-inl.h:1282: mode);
On 2011/12/07 17:02:47, Jakob wrote:
> nit: no need for line break

Done.

http://codereview.chromium.org/8820014/diff/4001/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/8820014/diff/4001/src/objects.cc#newcode8370
src/objects.cc:8370: void JSArray::Expand(int required_size) {
On 2011/12/07 17:02:47, Jakob wrote:
> Instead of all this code here, we should be able to make use of
> SetFastElementsCapacityAndLength (passing in the original length, and
> required_size as capacity).

Done.

http://codereview.chromium.org/8820014/diff/4001/test/mjsunit/array-construct...
File test/mjsunit/array-construct-transition.js (right):

http://codereview.chromium.org/8820014/diff/4001/test/mjsunit/array-construct...
test/mjsunit/array-construct-transition.js:33: a = new Array(0, 1, 2);
On 2011/12/07 17:02:47, Jakob wrote:
> nit: "var a", please. Same for b and c.

Done.

[Jakob Kummerow, 2011-12-08 15:25:58]

LGTM