Introduce FeedbackNexus for vector-based ics.

Introduce FeedbackNexus for vector-based ics.

A FeedbackNexus is the combination of a feedback vector, a slot(s) in
the vector, along with methods to query and manipulate that information
in a type-correct way.

A CallIC will have a CallICNexus, a LoadIC a LoadICNexus, etc.,
reflecting the fact that different types of ICs configure their data
in unique ways.

This CL limits itself to introducing and using the nexus type only for
CallICs. A follow-up will use them for Load and KeyedLoadICs for the
case when the --vector-ics flag is turned on.

The notion of a Nexus is also embedded at the lowest level of the IC
class. This makes sense because more ICs should become vector-based
in the future.

This CL is based on https://codereview.chromium.org/679073002/ which
should land first.

BUG=
R=ishell@chromium.org

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

[mvstanton, 2014-10-27 13:09:35]

Patchset #1 (id:1) has been deleted

[mvstanton, 2014-10-27 13:11:49]

Patchset #1 (id:20001) has been deleted

[mvstanton, 2014-10-27 13:12:55]

mvstanton@chromium.org changed reviewers:
+ ishell@chromium.org

[mvstanton, 2014-10-27 13:13:55]

Hi Igor,
Here is the FeedbackNexus, now in use for the production code CallIC. I will
delay the other nexus types (Load and KeyedLoad) to a future CL. Thanks for the
look,
--Michael

[mvstanton, 2014-10-27 16:31:27]

Patchset #2 (id:60001) has been deleted

[Igor Sheludko, 2014-10-28 11:12:36]

lgtm with nits:

https://codereview.chromium.org/680883004/diff/40001/src/ic/ic.cc
File src/ic/ic.cc (right):

https://codereview.chromium.org/680883004/diff/40001/src/ic/ic.cc#newcode545
src/ic/ic.cc:545: reinterpret_cast<CallICNexus*>(nexus));
static_cast?

https://codereview.chromium.org/680883004/diff/40001/src/ic/ic.cc#newcode569
src/ic/ic.cc:569: nexus->ConfigureUninitialized();
Does it make sense to proceed skipping write barriers for Uninitialized and
Generic sentinels?

https://codereview.chromium.org/680883004/diff/40001/src/ic/ic.cc#newcode2091
src/ic/ic.cc:2091: // A miss on a custom call ic always results in going
megamorphic.
This comment can be removed from here.

https://codereview.chromium.org/680883004/diff/40001/src/type-feedback-vector.cc
File src/type-feedback-vector.cc (right):

https://codereview.chromium.org/680883004/diff/40001/src/type-feedback-vector...
src/type-feedback-vector.cc:164: FixedArray::cast(*feedback)->length() !=
length) {
Do we need to allocate another array if existing one is longer than requested?
Does it make sense to right-trim and reuse the existing one?

https://codereview.chromium.org/680883004/diff/40001/src/type-feedback-vector...
src/type-feedback-vector.cc:227: 
Empty line is missing.

https://codereview.chromium.org/680883004/diff/40001/src/type-feedback-vector.h
File src/type-feedback-vector.h (right):

https://codereview.chromium.org/680883004/diff/40001/src/type-feedback-vector...
src/type-feedback-vector.h:205: void FindAllMaps(MapHandleList* maps) const {
ExtractMaps(maps); }
Do we really need this method? Why not just call ExtractMaps()?

[mvstanton, 2014-10-28 13:23:29]

Thanks Igor, here are the changes.
I used a template to avoid casting in IC[Utility]::Clear().

https://codereview.chromium.org/680883004/diff/40001/src/ic/ic.cc
File src/ic/ic.cc (right):

https://codereview.chromium.org/680883004/diff/40001/src/ic/ic.cc#newcode545
src/ic/ic.cc:545: reinterpret_cast<CallICNexus*>(nexus));
On 2014/10/28 11:12:36, Igor Sheludko wrote:
> static_cast?

I hated the cast anyway, so I've changed this to a template method where you
need to pass the appropriate kind of FeedbackNexus in order to compile. That
should be better, at least.

https://codereview.chromium.org/680883004/diff/40001/src/ic/ic.cc#newcode569
src/ic/ic.cc:569: nexus->ConfigureUninitialized();
On 2014/10/28 11:12:36, Igor Sheludko wrote:
> Does it make sense to proceed skipping write barriers for Uninitialized and
> Generic sentinels?

It does indeed, I've make that fix in type-feedback-vector.cc, thanks!

https://codereview.chromium.org/680883004/diff/40001/src/ic/ic.cc#newcode2091
src/ic/ic.cc:2091: // A miss on a custom call ic always results in going
megamorphic.
On 2014/10/28 11:12:36, Igor Sheludko wrote:
> This comment can be removed from here.

Done.

https://codereview.chromium.org/680883004/diff/40001/src/type-feedback-vector.cc
File src/type-feedback-vector.cc (right):

https://codereview.chromium.org/680883004/diff/40001/src/type-feedback-vector...
src/type-feedback-vector.cc:164: FixedArray::cast(*feedback)->length() !=
length) {
On 2014/10/28 11:12:36, Igor Sheludko wrote:
> Do we need to allocate another array if existing one is longer than requested?
> Does it make sense to right-trim and reuse the existing one?

Hmm, the only case that would happen would be if the IC is in POLYMORPHIC state
and it's going to a POLYMORPHIC state of lower degree or it's going to
MONOMORPHIC.

Neither of those happen in practice...we only go MONO->POLY, and to climb back
down to MONO there would be intervening states like UNINITIALIZED and/or
PREMONOMORPHIC, which are sentinels, so we'd lose the array in the meantime.

I think an optimization we may really want to do here soon is to share the
monomorphic arrays if possible between different sites.

https://codereview.chromium.org/680883004/diff/40001/src/type-feedback-vector...
src/type-feedback-vector.cc:227: 
On 2014/10/28 11:12:36, Igor Sheludko wrote:
> Empty line is missing.

Done.

https://codereview.chromium.org/680883004/diff/40001/src/type-feedback-vector.h
File src/type-feedback-vector.h (right):

https://codereview.chromium.org/680883004/diff/40001/src/type-feedback-vector...
src/type-feedback-vector.h:205: void FindAllMaps(MapHandleList* maps) const {
ExtractMaps(maps); }
On 2014/10/28 11:12:36, Igor Sheludko wrote:
> Do we really need this method? Why not just call ExtractMaps()?

Good point, removed.

[mvstanton, 2014-10-28 14:50:31]

Committed patchset #3 (id:100001) manually as 24945 (presubmit successful).