[turbofan] Call frequencies for JSCallFunction and JSCallConstruct.

[turbofan] Call frequencies for JSCallFunction and JSCallConstruct.

Extract the call counts from the type feedback vector during graph
building (either via the AstGraphBuilder or the BytecodeGraphBuilder),
and put them onto the JSCallFunction and JSCallConstruct operators,
so that they work even across inlinine through .apply and .call (which
was previously hacked by creating a temporary type feedback vector
for those).

The next logic step will be to make those call counts into real
relative call frequencies (also during graph building), so that we
can make inlining decisions that make sense for the function being
optimized (where absolute values are misleading).

R=jarin@chromium.org
BUG=v8:5267, v8:5372
Committed: https://crrev.com/0b8a69458ea25314f83d83fbe7f6ced4f903daf1
Cr-Commit-Position: refs/heads/master@{#39400}

[Benedikt Meurer, 2016-09-12 05:14:09]

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

[commit-bot: I haz the power, 2016-09-12 05:14:17]

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-12 05:15:40]

Hey Jaro,

Here's the first step towards useful frequencies for inlining. It is neutral
because it just uses the existing call counts as frequencies (which means it
doesn't fix the problem yet). The next step is to relativize those counts (which
will be an actual change in functionality).
Please take a look.

Thanks,
Benedikt

[commit-bot: I haz the power, 2016-09-12 05:38:41]

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

[commit-bot: I haz the power, 2016-09-12 05:38:42]

Dry run: Try jobs failed on following builders:
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/12493)
  v8_linux_dbg_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/b...)

[Benedikt Meurer, 2016-09-12 05:44:02]

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

[commit-bot: I haz the power, 2016-09-12 05:44:11]

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, 2016-09-12 06:06:19]

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

[commit-bot: I haz the power, 2016-09-12 06:06:20]

Dry run: This issue passed the CQ dry run.

[Benedikt Meurer, 2016-09-12 07:26:44]

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

[commit-bot: I haz the power, 2016-09-12 07:26:50]

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, 2016-09-12 07:50:37]

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

[commit-bot: I haz the power, 2016-09-12 07:50:37]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2016-09-12 09:05:40]

rmcilroy@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[rmcilroy, 2016-09-12 09:05:41]

https://codereview.chromium.org/2330883002/diff/40001/src/compiler/bytecode-g...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2330883002/diff/40001/src/compiler/bytecode-g...
src/compiler/bytecode-graph-builder.cc:1365: if (int const slot_index =
bytecode_iterator().GetIndexOperand(3)) {
Drive-by-nit. Not super keen on this getting the bytecode operand in the helper
function (particularly since we already pull out the feedback VectorSlotPair in
the caller). Could you pass through the feedback_extra slot to the helper
instead?

[Jarin, 2016-09-12 13:10:01]

lgtm.

https://codereview.chromium.org/2330883002/diff/40001/src/compiler/ast-graph-...
File src/compiler/ast-graph-builder.h (right):

https://codereview.chromium.org/2330883002/diff/40001/src/compiler/ast-graph-...
src/compiler/ast-graph-builder.h:268: float
ComputeCallFrequency(FeedbackVectorSlot slot) const;
Why not double?

https://codereview.chromium.org/2330883002/diff/40001/src/compiler/js-inlinin...
File src/compiler/js-inlining-heuristic.cc (right):

https://codereview.chromium.org/2330883002/diff/40001/src/compiler/js-inlinin...
src/compiler/js-inlining-heuristic.cc:278: return left.node < right.node;
Could we compare on node->id(), so that we do not depend on the whim of the zone
allocator (and on undefined behaviour).

[Benedikt Meurer, 2016-09-13 17:44:37]

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

[commit-bot: I haz the power, 2016-09-13 17:44:43]

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-13 17:45:07]

https://codereview.chromium.org/2330883002/diff/40001/src/compiler/ast-graph-...
File src/compiler/ast-graph-builder.h (right):

https://codereview.chromium.org/2330883002/diff/40001/src/compiler/ast-graph-...
src/compiler/ast-graph-builder.h:268: float
ComputeCallFrequency(FeedbackVectorSlot slot) const;
Because space. float fits perfectly for the two operators (fills the 32bit gap
on 64bit architectures) and offers more than enough precision for us.

https://codereview.chromium.org/2330883002/diff/40001/src/compiler/bytecode-g...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2330883002/diff/40001/src/compiler/bytecode-g...
src/compiler/bytecode-graph-builder.cc:1365: if (int const slot_index =
bytecode_iterator().GetIndexOperand(3)) {
On 2016/09/12 09:05:41, rmcilroy wrote:
> Drive-by-nit. Not super keen on this getting the bytecode operand in the
helper
> function (particularly since we already pull out the feedback VectorSlotPair
in
> the caller). Could you pass through the feedback_extra slot to the helper
> instead?

Done.

https://codereview.chromium.org/2330883002/diff/40001/src/compiler/js-inlinin...
File src/compiler/js-inlining-heuristic.cc (right):

https://codereview.chromium.org/2330883002/diff/40001/src/compiler/js-inlinin...
src/compiler/js-inlining-heuristic.cc:278: return left.node < right.node;
On 2016/09/12 13:10:01, Jarin wrote:
> Could we compare on node->id(), so that we do not depend on the whim of the
zone
> allocator (and on undefined behaviour).

Done.

[commit-bot: I haz the power, 2016-09-13 17:59:29]

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

[commit-bot: I haz the power, 2016-09-13 17:59:30]

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...)
  v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng_trigg...)

[Benedikt Meurer, 2016-09-13 17:59:38]

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

[commit-bot: I haz the power, 2016-09-13 17:59:50]

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, 2016-09-13 18:48:25]

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

[commit-bot: I haz the power, 2016-09-13 18:48:27]

Dry run: Try jobs failed on following builders:
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/12608)
  v8_linux_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng_triggered/b...)

[Benedikt Meurer, 2016-09-14 03:26:14]

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

[commit-bot: I haz the power, 2016-09-14 03:26:19]

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 04:09:39]

The CQ bit was unchecked by bmeurer@chromium.org

[Benedikt Meurer, 2016-09-14 04:09:47]

The CQ bit was checked by bmeurer@chromium.org

[Benedikt Meurer, 2016-09-14 04:09:47]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org
Link to the patchset: https://codereview.chromium.org/2330883002/#ps80001
(title: "Rebase onto the correct CL.")

[commit-bot: I haz the power, 2016-09-14 04:09:58]

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 04:12:30]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-14 04:13:14]

Description was changed from

==========
[turbofan] Call frequencies for JSCallFunction and JSCallConstruct.

Extract the call counts from the type feedback vector during graph
building (either via the AstGraphBuilder or the BytecodeGraphBuilder),
and put them onto the JSCallFunction and JSCallConstruct operators,
so that they work even across inlinine through .apply and .call (which
was previously hacked by creating a temporary type feedback vector
for those).

The next logic step will be to make those call counts into real
relative call frequencies (also during graph building), so that we
can make inlining decisions that make sense for the function being
optimized (where absolute values are misleading).

R=jarin@chromium.org
BUG=v8:5267,v8:5372
==========

to

==========
[turbofan] Call frequencies for JSCallFunction and JSCallConstruct.

Extract the call counts from the type feedback vector during graph
building (either via the AstGraphBuilder or the BytecodeGraphBuilder),
and put them onto the JSCallFunction and JSCallConstruct operators,
so that they work even across inlinine through .apply and .call (which
was previously hacked by creating a temporary type feedback vector
for those).

The next logic step will be to make those call counts into real
relative call frequencies (also during graph building), so that we
can make inlining decisions that make sense for the function being
optimized (where absolute values are misleading).

R=jarin@chromium.org
BUG=v8:5267,v8:5372

Committed: https://crrev.com/0b8a69458ea25314f83d83fbe7f6ced4f903daf1
Cr-Commit-Position: refs/heads/master@{#39400}
==========

[commit-bot: I haz the power, 2016-09-14 04:13:15]

Patchset 5 (id:??) landed as
https://crrev.com/0b8a69458ea25314f83d83fbe7f6ced4f903daf1
Cr-Commit-Position: refs/heads/master@{#39400}