[TypeFeedbackVector] Combine the literals array and the feedback vector.

[TypeFeedbackVector] Combine the literals array and the feedback vector.

They have the same lifetime. It's a match!

Both structures are native context dependent and dealt with (creation,
clearing, gathering feedback) at the same time. By treating the spaces used
for literal boilerplates as feedback vector slots, we no longer have to keep
track of the materialized literal count elsewhere.

A follow-on CL removes even more parser infrastructure related to this count. 

BUG=v8:5456
Review-Url: https://codereview.chromium.org/2655853010
Cr-Commit-Position: refs/heads/master@{#42771}
Committed: https://chromium.googlesource.com/v8/v8/+/93f05b64ae1c1f8e010467331de1bf02d800dede

[mvstanton, 2017-01-28 07:07:39]

The CQ bit was checked by mvstanton@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-28 07:07:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-28 07:12:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-28 07:12:14]

Dry run: Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)

[mvstanton, 2017-01-28 08:20:39]

The CQ bit was checked by mvstanton@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-28 08:20:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-28 08:52:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-28 08:52:45]

Dry run: This issue passed the CQ dry run.

[mvstanton, 2017-01-28 08:57:07]

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

[mvstanton, 2017-01-28 08:57:09]

Hi Benedikt,
Here is the combined feedback-vector / literals array that allows us to go
forward. Thanks for the look!
--Michael

[Benedikt Meurer, 2017-01-30 05:34:07]

Nice simplification! LGTM % nit from my side.

https://codereview.chromium.org/2655853010/diff/100001/src/compiler/js-create...
File src/compiler/js-create-lowering.h (right):

https://codereview.chromium.org/2655853010/diff/100001/src/compiler/js-create...
src/compiler/js-create-lowering.h:84: MaybeHandle<TypeFeedbackVector>
GetSpecializationLiterals(Node* node);
Nit: Rename to GetSpecializationTypeFeedbackVector please.

https://codereview.chromium.org/2655853010/diff/100001/src/type-feedback-vect...
File src/type-feedback-vector.cc (right):

https://codereview.chromium.org/2655853010/diff/100001/src/type-feedback-vect...
src/type-feedback-vector.cc:233: value = *undefined_value;
We should unify this to also use uninitialized_sentinel for uninitialized
LITERAL slots instead of using undefined there (could be follow-up CL).

[mvstanton, 2017-01-30 08:10:58]

mvstanton@chromium.org changed reviewers:
+ marja@chromium.org, mstarzinger@chromium.org

[mvstanton, 2017-01-30 08:10:58]

mvstanton@chromium.org changed required reviewers:
+ bmeurer@chromium.org, marja@chromium.org, mstarzinger@chromium.org

[mvstanton, 2017-01-30 08:10:59]

Hi Marja - could you have a look at my changes in parsing/? I think that this
work dovetails with something you wanted to do. It's fantastic that we don't
need literal counts anymore. :)

Michael - could you verify heap/? Yay, less roots. :D

Thanks,
--Michael

[marja, 2017-01-30 09:03:01]

ast/* and parsing/* LGTM

As discussed offline, the
function_state_->NextMaterializedLiteralIndex();
things will go away in the follow-up CL.

[mvstanton, 2017-01-30 10:50:30]

Description was changed from

==========
[TypeFeedbackVector] Combine the literals array and the feedback vector.

They have the same lifetime. It's a match!

BUG=
==========

to

==========
[TypeFeedbackVector] Combine the literals array and the feedback vector.

They have the same lifetime. It's a match!

Both structures are native context dependent and dealt with (creation, clearing,
gathering feedback) at the same time. By treating the spaces used for literal
boilerplates as feedback vector slots, we no longer have to keep track of the
materialized literal count elsewhere.

A follow-on CL removes even more parser infrastructure related to this count. 

BUG=v8:5456
==========

[mvstanton, 2017-01-30 10:53:04]

Description was changed from

==========
[TypeFeedbackVector] Combine the literals array and the feedback vector.

They have the same lifetime. It's a match!

Both structures are native context dependent and dealt with (creation, clearing,
gathering feedback) at the same time. By treating the spaces used for literal
boilerplates as feedback vector slots, we no longer have to keep track of the
materialized literal count elsewhere.

A follow-on CL removes even more parser infrastructure related to this count. 

BUG=v8:5456
==========

to

==========
[TypeFeedbackVector] Combine the literals array and the feedback vector.

They have the same lifetime. It's a match!

Both structures are native context dependent and dealt with (creation,
clearing, gathering feedback) at the same time. By treating the spaces used
for literal boilerplates as feedback vector slots, we no longer have to keep
track of the materialized literal count elsewhere.

A follow-on CL removes even more parser infrastructure related to this count. 

BUG=v8:5456
==========

[mvstanton, 2017-01-30 10:56:41]

Thanks Benedikt, addressed...

https://codereview.chromium.org/2655853010/diff/100001/src/compiler/js-create...
File src/compiler/js-create-lowering.h (right):

https://codereview.chromium.org/2655853010/diff/100001/src/compiler/js-create...
src/compiler/js-create-lowering.h:84: MaybeHandle<TypeFeedbackVector>
GetSpecializationLiterals(Node* node);
On 2017/01/30 05:34:05, Benedikt Meurer wrote:
> Nit: Rename to GetSpecializationTypeFeedbackVector please.

Done.

https://codereview.chromium.org/2655853010/diff/100001/src/type-feedback-vect...
File src/type-feedback-vector.cc (right):

https://codereview.chromium.org/2655853010/diff/100001/src/type-feedback-vect...
src/type-feedback-vector.cc:233: value = *undefined_value;
On 2017/01/30 05:34:05, Benedikt Meurer wrote:
> We should unify this to also use uninitialized_sentinel for uninitialized
> LITERAL slots instead of using undefined there (could be follow-up CL).

Yep, excellent idea - I will follow up with that.

[mvstanton, 2017-01-30 11:04:41]

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

[mvstanton, 2017-01-30 11:04:41]

mvstanton@chromium.org changed required reviewers:
+ yangguo@chromium.org

[mvstanton, 2017-01-30 11:04:42]

Hi Yang, if you could have a look at the partial serializer change that would be
awesome. Thanks!
--Michael

[Michael Starzinger, 2017-01-30 11:16:45]

LGTM. Just nits.

https://codereview.chromium.org/2655853010/diff/100001/src/ast/ast.h
File src/ast/ast.h (right):

https://codereview.chromium.org/2655853010/diff/100001/src/ast/ast.h#newcode1243
src/ast/ast.h:1243: // Base class for literals that needs space in the
corresponding JSFunction.
nit: s/in the corresponding JSFunction/in the type feedback vector/

https://codereview.chromium.org/2655853010/diff/100001/src/ast/ast.h#newcode1533
src/ast/ast.h:1533: void AssignFeedbackVectorSlots(FeedbackVectorSpec* spec,
nit: Shouldn't this be the same as just using the inherited version of the
method?

https://codereview.chromium.org/2655853010/diff/100001/src/compiler/js-create...
File src/compiler/js-create-lowering.h (right):

https://codereview.chromium.org/2655853010/diff/100001/src/compiler/js-create...
src/compiler/js-create-lowering.h:84: MaybeHandle<TypeFeedbackVector>
GetSpecializationLiterals(Node* node);
On 2017/01/30 05:34:05, Benedikt Meurer wrote:
> Nit: Rename to GetSpecializationTypeFeedbackVector please.

+1

https://codereview.chromium.org/2655853010/diff/140001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2655853010/diff/140001/src/objects.h#newcode899
src/objects.h:899: V(LITERALS_ARRAY_SUB_TYPE)                     \
nit: I think we can drop this sub-type now.

[Yang, 2017-01-30 11:40:21]

On 2017/01/30 11:16:45, Michael Starzinger wrote:
> LGTM. Just nits.
> 
> https://codereview.chromium.org/2655853010/diff/100001/src/ast/ast.h
> File src/ast/ast.h (right):
> 
>
https://codereview.chromium.org/2655853010/diff/100001/src/ast/ast.h#newcode1243
> src/ast/ast.h:1243: // Base class for literals that needs space in the
> corresponding JSFunction.
> nit: s/in the corresponding JSFunction/in the type feedback vector/
> 
>
https://codereview.chromium.org/2655853010/diff/100001/src/ast/ast.h#newcode1533
> src/ast/ast.h:1533: void AssignFeedbackVectorSlots(FeedbackVectorSpec* spec,
> nit: Shouldn't this be the same as just using the inherited version of the
> method?
> 
>
https://codereview.chromium.org/2655853010/diff/100001/src/compiler/js-create...
> File src/compiler/js-create-lowering.h (right):
> 
>
https://codereview.chromium.org/2655853010/diff/100001/src/compiler/js-create...
> src/compiler/js-create-lowering.h:84: MaybeHandle<TypeFeedbackVector>
> GetSpecializationLiterals(Node* node);
> On 2017/01/30 05:34:05, Benedikt Meurer wrote:
> > Nit: Rename to GetSpecializationTypeFeedbackVector please.
> 
> +1
> 
> https://codereview.chromium.org/2655853010/diff/140001/src/objects.h
> File src/objects.h (right):
> 
>
https://codereview.chromium.org/2655853010/diff/140001/src/objects.h#newcode899
> src/objects.h:899: V(LITERALS_ARRAY_SUB_TYPE)                     \
> nit: I think we can drop this sub-type now.

lgtm.

[mvstanton, 2017-01-30 11:56:04]

The CQ bit was checked by mvstanton@chromium.org

[mvstanton, 2017-01-30 11:56:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, marja@chromium.org, mstarzinger@chromium.org,
yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/2655853010/#ps160001
(title: "more comments.")

[commit-bot: I haz the power, 2017-01-30 11:56:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mvstanton, 2017-01-30 11:57:04]

Thanks all, landing...

https://codereview.chromium.org/2655853010/diff/100001/src/ast/ast.h
File src/ast/ast.h (right):

https://codereview.chromium.org/2655853010/diff/100001/src/ast/ast.h#newcode1243
src/ast/ast.h:1243: // Base class for literals that needs space in the
corresponding JSFunction.
On 2017/01/30 11:16:44, Michael Starzinger wrote:
> nit: s/in the corresponding JSFunction/in the type feedback vector/

Done.

https://codereview.chromium.org/2655853010/diff/100001/src/ast/ast.h#newcode1533
src/ast/ast.h:1533: void AssignFeedbackVectorSlots(FeedbackVectorSpec* spec,
On 2017/01/30 11:16:44, Michael Starzinger wrote:
> nit: Shouldn't this be the same as just using the inherited version of the
> method?

Yep, thanks!

https://codereview.chromium.org/2655853010/diff/140001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2655853010/diff/140001/src/objects.h#newcode899
src/objects.h:899: V(LITERALS_ARRAY_SUB_TYPE)                     \
On 2017/01/30 11:16:44, Michael Starzinger wrote:
> nit: I think we can drop this sub-type now.

Done.

[commit-bot: I haz the power, 2017-01-30 12:31:37]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1485777364780550,
"parent_rev": "a15e9bfd3063792817b31feaddc8fe5a4d6921c2", "commit_rev":
"93f05b64ae1c1f8e010467331de1bf02d800dede"}

[commit-bot: I haz the power, 2017-01-30 12:31:42]

Description was changed from

==========
[TypeFeedbackVector] Combine the literals array and the feedback vector.

They have the same lifetime. It's a match!

Both structures are native context dependent and dealt with (creation,
clearing, gathering feedback) at the same time. By treating the spaces used
for literal boilerplates as feedback vector slots, we no longer have to keep
track of the materialized literal count elsewhere.

A follow-on CL removes even more parser infrastructure related to this count. 

BUG=v8:5456
==========

to

==========
[TypeFeedbackVector] Combine the literals array and the feedback vector.

They have the same lifetime. It's a match!

Both structures are native context dependent and dealt with (creation,
clearing, gathering feedback) at the same time. By treating the spaces used
for literal boilerplates as feedback vector slots, we no longer have to keep
track of the materialized literal count elsewhere.

A follow-on CL removes even more parser infrastructure related to this count. 

BUG=v8:5456

Review-Url: https://codereview.chromium.org/2655853010
Cr-Commit-Position: refs/heads/master@{#42771}
Committed:
https://chromium.googlesource.com/v8/v8/+/93f05b64ae1c1f8e010467331de1bf02d80...
==========

[commit-bot: I haz the power, 2017-01-30 12:31:44]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/v8/v8/+/93f05b64ae1c1f8e010467331de1bf02d80...