CallIC customization stubs must accept that a vector slot is cleared.

CallIC customization stubs must accept that a vector slot is cleared.

The CallIC Array custom IC stub read from the type vector, expecting
to get an AllocationSite. But there are paths in the system where a
type vector can be re-created with default values, even though we
currently grant an exception to clearing of vector slots with
AllocationSites in them at gc time.

BUG=392114
LOG=N
R=verwaest@chromium.org

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

[mvstanton, 2014-07-24 15:11:25]

Hi Toon,
Here is the fix for the chrome bug we discussed.

[Toon Verwaest, 2014-07-28 14:11:39]

lgtm with nit.
You can land the repro separately.

https://codereview.chromium.org/418023002/diff/1/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

https://codereview.chromium.org/418023002/diff/1/src/arm/code-stubs-arm.cc#ne...
src/arm/code-stubs-arm.cc:2972: __ ldr(r5, FieldMemOperand(r4,
AllocationSite::kMapOffset));
HeapObject::kMapOffset?

https://codereview.chromium.org/418023002/diff/1/src/arm64/code-stubs-arm64.cc
File src/arm64/code-stubs-arm64.cc (right):

https://codereview.chromium.org/418023002/diff/1/src/arm64/code-stubs-arm64.c...
src/arm64/code-stubs-arm64.cc:3241: __ Ldr(map, FieldMemOperand(scratch,
AllocationSite::kMapOffset));
HeapObject::kMapOffset?
(You don't know yet whether it's an AllocationSite, just that it's a HeapObject)

https://codereview.chromium.org/418023002/diff/1/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

https://codereview.chromium.org/418023002/diff/1/src/ia32/code-stubs-ia32.cc#...
src/ia32/code-stubs-ia32.cc:2383: __ cmp(FieldOperand(ecx, 0),
factory->allocation_site_map());
HeapObject::kMapoffset?

https://codereview.chromium.org/418023002/diff/1/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

https://codereview.chromium.org/418023002/diff/1/src/x64/code-stubs-x64.cc#ne...
src/x64/code-stubs-x64.cc:2262: __ Cmp(FieldOperand(rcx, 0),
factory->allocation_site_map());
HeapObject::kMapOffset?

[mvstanton, 2014-07-28 16:02:50]

Thanks Toon, here is the update we discussed.

The custom handler still has to check for an allocation site, as it's not the
sole controller of the vector slot.

The default handler does some extra checking when it's planning to stomp the
slot with the megamorphic sentinel. It will only stomp if there is a JSFunction
in place, otherwise it MISSes to the runtime for more deft handling.

By fiat, I declare that CallIC vector slots can't contain smis. They don't
currently, and in the next customization opportunities I don't need one. That
way, we don't have to check for the smi type. I added some asserts around this.

https://codereview.chromium.org/418023002/diff/1/src/arm64/code-stubs-arm64.cc
File src/arm64/code-stubs-arm64.cc (right):

https://codereview.chromium.org/418023002/diff/1/src/arm64/code-stubs-arm64.c...
src/arm64/code-stubs-arm64.cc:3241: __ Ldr(map, FieldMemOperand(scratch,
AllocationSite::kMapOffset));
On 2014/07/28 14:11:38, Toon Verwaest wrote:
> HeapObject::kMapOffset?
> (You don't know yet whether it's an AllocationSite, just that it's a
HeapObject)

Right, we will store in this slot the uninitialized symbol, the megamorphic
symbol, a function pointer or an allocationsite. Since those are all heap
objects with maps, I can dereference here. Changed to HeapObject::kMapOffset.

https://codereview.chromium.org/418023002/diff/1/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

https://codereview.chromium.org/418023002/diff/1/src/x64/code-stubs-x64.cc#ne...
src/x64/code-stubs-x64.cc:2262: __ Cmp(FieldOperand(rcx, 0),
factory->allocation_site_map());
On 2014/07/28 14:11:38, Toon Verwaest wrote:
> HeapObject::kMapOffset?

Done.

[Toon Verwaest, 2014-07-29 11:28:18]

lgtm with nit

https://codereview.chromium.org/418023002/diff/60001/test/mjsunit/regress/reg...
File test/mjsunit/regress/regress-392114.js (right):

https://codereview.chromium.org/418023002/diff/60001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-392114.js:38: if (arg === true) {
if (arg) { %DeoptimizeFunction(function); }

[mvstanton, 2014-07-29 11:53:38]

Committed patchset #3 manually as r22668.