Moved type feedback vector to SharedFunctionInfo.

Moved type feedback vector to SharedFunctionInfo.

Type Vector followup: the type vector currently lives off the code object. This CL moves it to the SharedFunctionInfo, facilitating re-use and continued use in crankshafted code if desired.

R=bmeurer@chromium.org

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

[mvstanton, 2014-03-03 16:47:19]

Hi Benedikt, could you have a look? Thanks!
--Michael

[Benedikt Meurer, 2014-03-04 10:02:50]

As mentioned offline, make sure to not clear existing type feedback when full
codegen is run again prior to crankshaft. Also add a test case to assert this.

[mvstanton, 2014-03-04 13:50:12]

Hi Benedikt,
Good advice. I have a new test, and was generally able to extract initialization
of the type vector from full code-gen ast node visiting at all, though I had to
alter the sentinel used for the ForIn statement to make this graceful.

On recompilation, care is taken to use the feedback vector from the shared
function info, rather than creating one afresh.
Thanks,
--Michael

[Benedikt Meurer, 2014-03-05 06:23:57]

Since this is unrelated to the SharedFunctionInfo move, can we please split this
into two separate CLs?

[Benedikt Meurer, 2014-03-05 07:22:55]

Hey Michael,
As discussed offline, it makes sense to keep this as one single CL.
Nice refactoring, just a few comments/nits.
-- Benedikt

https://codereview.chromium.org/178463007/diff/80001/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/178463007/diff/80001/src/compiler.cc#newcode264
src/compiler.cc:264: *TypeFeedbackInfo::UninitializedSentinel(isolate()));
Nit: indentation

https://codereview.chromium.org/178463007/diff/80001/src/compiler.cc#newcode267
src/compiler.cc:267: feedback_vector_ =
isolate()->factory()->NewFixedArrayWithHoles(length,
Nit: put arguments on next line.

https://codereview.chromium.org/178463007/diff/80001/src/compiler.cc#newcode592
src/compiler.cc:592: shared->set_feedback_vector(*(info->feedback_vector()));
Nit: *info->feedback_vector()

https://codereview.chromium.org/178463007/diff/80001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/178463007/diff/80001/src/hydrogen.cc#newcode7228
src/hydrogen.cc:7228:
target_shared->set_feedback_vector(*(target_info.feedback_vector()));
Nit: *target_info.feedback_vector()

https://codereview.chromium.org/178463007/diff/80001/src/objects-printer.cc
File src/objects-printer.cc (right):

https://codereview.chromium.org/178463007/diff/80001/src/objects-printer.cc#n...
src/objects-printer.cc:880: feedback_vector()->ShortPrint(out);
Why don't you use FixedArrayPrint() here?

https://codereview.chromium.org/178463007/diff/80001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/178463007/diff/80001/src/objects.cc#newcode10655
src/objects.cc:10655: if (!obj->IsAllocationSite()) {
Unrelated to this CL, but... Do we really want to keep all these special cases
for allocation sites?

https://codereview.chromium.org/178463007/diff/80001/src/objects.cc#newcode10657
src/objects.cc:10657: TypeFeedbackInfo::RawUninitializedSentinel(heap));
Nit: Please add a comment that this ASSERT is only necessary for the
SKIP_WRITE_BARRIER below.

https://codereview.chromium.org/178463007/diff/80001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/178463007/diff/80001/src/runtime.cc#newcode8562
src/runtime.cc:8562: function->shared()->ClearTypeFeedbackInfo(isolate->heap());
This should be done unconditionally.

[mvstanton, 2014-03-05 08:48:02]

Thx for the review/discussion, here are updates,
--Michael

https://codereview.chromium.org/178463007/diff/80001/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/178463007/diff/80001/src/compiler.cc#newcode264
src/compiler.cc:264: *TypeFeedbackInfo::UninitializedSentinel(isolate()));
On 2014/03/05 07:22:56, Benedikt Meurer wrote:
> Nit: indentation

Done.

https://codereview.chromium.org/178463007/diff/80001/src/compiler.cc#newcode267
src/compiler.cc:267: feedback_vector_ =
isolate()->factory()->NewFixedArrayWithHoles(length,
On 2014/03/05 07:22:56, Benedikt Meurer wrote:
> Nit: put arguments on next line.

Done.

https://codereview.chromium.org/178463007/diff/80001/src/compiler.cc#newcode592
src/compiler.cc:592: shared->set_feedback_vector(*(info->feedback_vector()));
On 2014/03/05 07:22:56, Benedikt Meurer wrote:
> Nit: *info->feedback_vector()

Done.

https://codereview.chromium.org/178463007/diff/80001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/178463007/diff/80001/src/hydrogen.cc#newcode7228
src/hydrogen.cc:7228:
target_shared->set_feedback_vector(*(target_info.feedback_vector()));
On 2014/03/05 07:22:56, Benedikt Meurer wrote:
> Nit: *target_info.feedback_vector()

Done.

https://codereview.chromium.org/178463007/diff/80001/src/objects-printer.cc
File src/objects-printer.cc (right):

https://codereview.chromium.org/178463007/diff/80001/src/objects-printer.cc#n...
src/objects-printer.cc:880: feedback_vector()->ShortPrint(out);
On 2014/03/05 07:22:56, Benedikt Meurer wrote:
> Why don't you use FixedArrayPrint() here?

good idea, thx.

https://codereview.chromium.org/178463007/diff/80001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/178463007/diff/80001/src/objects.cc#newcode10655
src/objects.cc:10655: if (!obj->IsAllocationSite()) {
On 2014/03/05 07:22:56, Benedikt Meurer wrote:
> Unrelated to this CL, but... Do we really want to keep all these special cases
> for allocation sites?

Indeed, per our discussion, I'll try and remove the special case in another CL.

https://codereview.chromium.org/178463007/diff/80001/src/objects.cc#newcode10657
src/objects.cc:10657: TypeFeedbackInfo::RawUninitializedSentinel(heap));
On 2014/03/05 07:22:56, Benedikt Meurer wrote:
> Nit: Please add a comment that this ASSERT is only necessary for the
> SKIP_WRITE_BARRIER below.

Done.

https://codereview.chromium.org/178463007/diff/80001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/178463007/diff/80001/src/runtime.cc#newcode8562
src/runtime.cc:8562: function->shared()->ClearTypeFeedbackInfo(isolate->heap());
On 2014/03/05 07:22:56, Benedikt Meurer wrote:
> This should be done unconditionally.

Done.

[Benedikt Meurer, 2014-03-05 11:32:24]

LGTM

[mvstanton, 2014-03-07 10:12:34]

Committed patchset #5 manually as r19712 (presubmit successful).