[turbofan] Collect invocation counts and compute relative call frequencies.

[turbofan] Collect invocation counts and compute relative call frequencies.

Add a notion of "invocation count" to the baseline compilers, which
increment a special slot in the TypeFeedbackVector for each invocation
of a given function (the optimized code doesn't currently collect this
information).

Use this invocation count to relativize the call counts on the call
sites within the function, so that the inlining heuristic has a view
of relative importance of a call site rather than some absolute numbers
with unclear meaning for the current function. Also apply the call site
frequency as a factor to all frequencies in the inlinee by passing this
to the graph builders so that the importance of a call site in an
inlinee is relative to the topmost optimized function.

Note that all functions that neither have literals nor need type
feedback slots will share a single invocation count cell in the
canonical empty type feedback vector, so their invocation count is
meaningless, but that doesn't matter since we only use the invocation
count to relativize call counts within the function, which we only have
if we have at least one type feedback vector (the CallIC slot).

See the design document for additional details on this change:
https://docs.google.com/document/d/1VoYBhpDhJC4VlqMXCKvae-8IGuheBGxy32EOgC2LnT8

BUG=v8:5267, v8:5372
R=mvstanton@chromium.org,rmcilroy@chromium.org,mstarzinger@chromium.org

Committed: https://crrev.com/c7d7ca361dd1ba399becd54ce0d9c6e53f76c870
Cr-Commit-Position: refs/heads/master@{#39410}

[Benedikt Meurer, 2016-09-14 06:42:15]

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

[commit-bot: I haz the power, 2016-09-14 06:42:18]

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

[Benedikt Meurer, 2016-09-14 06:43:41]

Hey guys,

This is the change described in go/v8-turbofan-inlining-heuristics, which allows
us to do inlining based on relative importance rather than absolute numbers that
don't necessarily make sense in the context of the topmost optimized function.
Please take a look.

Thanks,
Benedikt

[Benedikt Meurer, 2016-09-14 06:59:17]

Note that this change will allow escape analysis to boost RayTrace performance
by 15-20% once it's enabled.

[commit-bot: I haz the power, 2016-09-14 07:08:52]

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

[commit-bot: I haz the power, 2016-09-14 07:08:53]

Dry run: This issue passed the CQ dry run.

[Michael Starzinger, 2016-09-14 07:53:45]

LGTM from my end. Please wait on a review from Michael Stanton about the changes
to the type feedback vector as well.

https://codereview.chromium.org/2337123003/diff/1/src/full-codegen/arm/full-c...
File src/full-codegen/arm/full-codegen-arm.cc (right):

https://codereview.chromium.org/2337123003/diff/1/src/full-codegen/arm/full-c...
src/full-codegen/arm/full-codegen-arm.cc:132: __ ldr(r2, FieldMemOperand(r1,
JSFunction::kLiteralsOffset));
suggestion: We could even add a code comment (the ones visible in the
disassembly via --code-comments) for this block.

[mvstanton, 2016-09-14 09:26:09]

Looks good. I have a couple of comments, can be addressed now or in a follow up
CL. LGTM.

https://codereview.chromium.org/2337123003/diff/1/src/compiler/ast-graph-buil...
File src/compiler/ast-graph-builder.cc (right):

https://codereview.chromium.org/2337123003/diff/1/src/compiler/ast-graph-buil...
src/compiler/ast-graph-builder.cc:3111: invocation_frequency_;
How about giving the CallICNexus responsibility for this computation...it can
see the invocation_count() after all.

https://codereview.chromium.org/2337123003/diff/1/src/compiler/bytecode-graph...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2337123003/diff/1/src/compiler/bytecode-graph...
src/compiler/bytecode-graph-builder.cc:1361: return
static_cast<float>(call_count / invocation_count) *
Here too.

https://codereview.chromium.org/2337123003/diff/1/src/full-codegen/arm/full-c...
File src/full-codegen/arm/full-codegen-arm.cc (right):

https://codereview.chromium.org/2337123003/diff/1/src/full-codegen/arm/full-c...
src/full-codegen/arm/full-codegen-arm.cc:132: __ ldr(r2, FieldMemOperand(r1,
JSFunction::kLiteralsOffset));
On 2016/09/14 07:53:45, Michael Starzinger wrote:
> suggestion: We could even add a code comment (the ones visible in the
> disassembly via --code-comments) for this block.

Let's certainly do so!

[rmcilroy, 2016-09-14 09:30:07]

Bytecode related parts LGTM, thanks.

[Benedikt Meurer, 2016-09-14 09:52:41]

The CQ bit was checked by bmeurer@chromium.org

[Benedikt Meurer, 2016-09-14 09:52:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
mvstanton@chromium.org, mstarzinger@chromium.org, rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/2337123003/#ps20001
(title: "Address feedback.")

[commit-bot: I haz the power, 2016-09-14 09:52:53]

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, 2016-09-14 10:20:13]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-14 10:20:57]

Description was changed from

==========
[turbofan] Collect invocation counts and compute relative call frequencies.

Add a notion of "invocation count" to the baseline compilers, which
increment a special slot in the TypeFeedbackVector for each invocation
of a given function (the optimized code doesn't currently collect this
information).

Use this invocation count to relativize the call counts on the call
sites within the function, so that the inlining heuristic has a view
of relative importance of a call site rather than some absolute numbers
with unclear meaning for the current function. Also apply the call site
frequency as a factor to all frequencies in the inlinee by passing this
to the graph builders so that the importance of a call site in an
inlinee is relative to the topmost optimized function.

Note that all functions that neither have literals nor need type
feedback slots will share a single invocation count cell in the
canonical empty type feedback vector, so their invocation count is
meaningless, but that doesn't matter since we only use the invocation
count to relativize call counts within the function, which we only have
if we have at least one type feedback vector (the CallIC slot).

See the design document for additional details on this change:
https://docs.google.com/document/d/1VoYBhpDhJC4VlqMXCKvae-8IGuheBGxy32EOgC2LnT8

BUG=v8:5267,v8:5372
R=mvstanton@chromium.org,rmcilroy@chromium.org,mstarzinger@chromium.org
==========

to

==========
[turbofan] Collect invocation counts and compute relative call frequencies.

Add a notion of "invocation count" to the baseline compilers, which
increment a special slot in the TypeFeedbackVector for each invocation
of a given function (the optimized code doesn't currently collect this
information).

Use this invocation count to relativize the call counts on the call
sites within the function, so that the inlining heuristic has a view
of relative importance of a call site rather than some absolute numbers
with unclear meaning for the current function. Also apply the call site
frequency as a factor to all frequencies in the inlinee by passing this
to the graph builders so that the importance of a call site in an
inlinee is relative to the topmost optimized function.

Note that all functions that neither have literals nor need type
feedback slots will share a single invocation count cell in the
canonical empty type feedback vector, so their invocation count is
meaningless, but that doesn't matter since we only use the invocation
count to relativize call counts within the function, which we only have
if we have at least one type feedback vector (the CallIC slot).

See the design document for additional details on this change:
https://docs.google.com/document/d/1VoYBhpDhJC4VlqMXCKvae-8IGuheBGxy32EOgC2LnT8

BUG=v8:5267,v8:5372
R=mvstanton@chromium.org,rmcilroy@chromium.org,mstarzinger@chromium.org

Committed: https://crrev.com/c7d7ca361dd1ba399becd54ce0d9c6e53f76c870
Cr-Commit-Position: refs/heads/master@{#39410}
==========

[commit-bot: I haz the power, 2016-09-14 10:20:59]

Patchset 2 (id:??) landed as
https://crrev.com/c7d7ca361dd1ba399becd54ce0d9c6e53f76c870
Cr-Commit-Position: refs/heads/master@{#39410}

[Benedikt Meurer, 2016-09-14 10:55:45]

https://codereview.chromium.org/2337123003/diff/1/src/compiler/ast-graph-buil...
File src/compiler/ast-graph-builder.cc (right):

https://codereview.chromium.org/2337123003/diff/1/src/compiler/ast-graph-buil...
src/compiler/ast-graph-builder.cc:3111: invocation_frequency_;
On 2016/09/14 09:26:09, mvstanton wrote:
> How about giving the CallICNexus responsibility for this computation...it can
> see the invocation_count() after all.

Done.

https://codereview.chromium.org/2337123003/diff/1/src/compiler/bytecode-graph...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2337123003/diff/1/src/compiler/bytecode-graph...
src/compiler/bytecode-graph-builder.cc:1361: return
static_cast<float>(call_count / invocation_count) *
On 2016/09/14 09:26:09, mvstanton wrote:
> Here too.

Done.

https://codereview.chromium.org/2337123003/diff/1/src/full-codegen/arm/full-c...
File src/full-codegen/arm/full-codegen-arm.cc (right):

https://codereview.chromium.org/2337123003/diff/1/src/full-codegen/arm/full-c...
src/full-codegen/arm/full-codegen-arm.cc:132: __ ldr(r2, FieldMemOperand(r1,
JSFunction::kLiteralsOffset));
On 2016/09/14 07:53:45, Michael Starzinger wrote:
> suggestion: We could even add a code comment (the ones visible in the
> disassembly via --code-comments) for this block.

Done.