Array constructor shouldn't require a Cell, just an AllocationSite.

Array constructor shouldn't require a Cell, just an AllocationSite.

The Array constructor has a needless dependency on an input argument
that is a Cell. It uses this to walk through to an AllocationSite.
The dependency hampers future work. Instead, pass the AllocationSite
as input to the Array constructor.

R=bmeurer@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=18716

[mvstanton, 2014-01-20 12:19:37]

Hi Benedikt, here is a CL that removes a dependency on a type cell in the other
direction. No hurry, thanks for a look when you can,
--Michael

[Benedikt Meurer, 2014-01-20 12:24:21]

https://codereview.chromium.org/140963004/diff/30001/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (left):

https://codereview.chromium.org/140963004/diff/30001/src/arm/code-stubs-arm.c...
src/arm/code-stubs-arm.cc:5627: __ CompareRoot(r5,
Heap::kAllocationSiteMapRootIndex);
Can we please keep this check?

https://codereview.chromium.org/140963004/diff/30001/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (left):

https://codereview.chromium.org/140963004/diff/30001/src/ia32/code-stubs-ia32...
src/ia32/code-stubs-ia32.cc:5479: __ cmp(FieldOperand(ecx, 0),
Immediate(allocation_site_map));
Same.

https://codereview.chromium.org/140963004/diff/30001/src/mips/code-stubs-mips.cc
File src/mips/code-stubs-mips.cc (left):

https://codereview.chromium.org/140963004/diff/30001/src/mips/code-stubs-mips...
src/mips/code-stubs-mips.cc:5796: __ LoadRoot(at,
Heap::kAllocationSiteMapRootIndex);
Same.

https://codereview.chromium.org/140963004/diff/30001/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (left):

https://codereview.chromium.org/140963004/diff/30001/src/x64/code-stubs-x64.c...
src/x64/code-stubs-x64.cc:5285: __ Cmp(FieldOperand(rcx, 0),
allocation_site_map);
Same.

[mvstanton, 2014-01-20 13:36:11]

Updated, thx for comments!
--Michael

https://codereview.chromium.org/140963004/diff/30001/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (left):

https://codereview.chromium.org/140963004/diff/30001/src/arm/code-stubs-arm.c...
src/arm/code-stubs-arm.cc:5627: __ CompareRoot(r5,
Heap::kAllocationSiteMapRootIndex);
On 2014/01/20 12:24:22, Benedikt Meurer wrote:
> Can we please keep this check?

Sure thing, done for all platforms.

[Benedikt Meurer, 2014-01-21 06:32:39]

LGTM, just one semantic comment

https://codereview.chromium.org/140963004/diff/140001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/140963004/diff/140001/src/runtime.cc#newcode1...
src/runtime.cc:14783: type_info->IsAllocationSite()) {
Maybe I'm missing something, but if the type_info is not undefined, then it has
to be an AllocationSite, right? So we should probably turn the
IsAllocationSite() test into an ASSERT here?

[mvstanton, 2014-01-21 15:58:18]

Okay, thanks very much,
--Michael

https://codereview.chromium.org/140963004/diff/140001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/140963004/diff/140001/src/runtime.cc#newcode1...
src/runtime.cc:14783: type_info->IsAllocationSite()) {
On 2014/01/21 06:32:39, Benedikt Meurer wrote:
> Maybe I'm missing something, but if the type_info is not undefined, then it
has
> to be an AllocationSite, right? So we should probably turn the
> IsAllocationSite() test into an ASSERT here?

I agree. I can just remove the line because
Handle<AllocationSite>::cast(type_info) does that in debug mode.

[mvstanton, 2014-01-21 16:04:50]

Committed patchset #4 manually as r18716 (presubmit successful).