[turbofan] Initial support for polymorphic inlining.

[turbofan] Initial support for polymorphic inlining.

For call sites where the target is not a known constant, but potentially
a list of known constants (i.e. a Phi with all HeapConstant inputs), we
still record the call site as a potential candidate for inlining.
In case the heuristic picks that candidate for inlining, we
expand the call site to a dispatched call site and invoke the
actual inlining logic for all the nested call sites.

Like Crankshaft, we currently allow up to 4 targets for polymorphic inlining,
although we might want to refine that later.

This approach is different from what Crankshaft does in
that we don't duplicate the evaluation of the parameters per polymorphic
case. Instead we first perform the load of the target (which usually
dispatches based on the receiver map), then we evaluate all the
parameters, and then we dispatch again based on the known targets. This
might generate better or worse code compared to what Crankshaft does,
and for the cases where we generate worse code (i.e. because we have
only trivial parameters or no parameters at all), we might want to
investigate optimizing away the double dispatch in the
future.

R=mvstanton@chromium.org
BUG=v8:5267, v8:5365
Committed: https://crrev.com/7d4ab7d43e335f36b0f91eb36e231dc7bea048ea
Cr-Commit-Position: refs/heads/master@{#39302}

[Benedikt Meurer, 2016-09-09 05:18:32]

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

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

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

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

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

Dry run: This issue passed the CQ dry run.

[Benedikt Meurer, 2016-09-09 07:01:42]

Description was changed from

==========
[turbofan] Initial support for polymorphic inlining.

For call sites where the target is not a known constant, but potentially
a list of known constants (i.e. a Phi with all HeapConstant inputs), we
do still record the call site as a potential candidate for inlining now
and in case the heuristic picks that candidate for inlining, we first
expand the call site to a dispatched call site and then invoke the
actual inlining logic for all the nested call sites.

We currently allow up to 4 targets for polymorphic inlining, just like
Crankshaft does, although we might want to refine that later.

Also note that this approach is different from what Crankshaft does in
that we don't duplicate the evaluation of the parameters per polymorphic
case, but instead we first perform the load of the target (which usually
dispatches based on the receiver map), then we evaluate all the
parameters, and then we dispatch again based on the known targets. This
might generate better or worse code compared to what Crankshaft does,
and for the cases where we generate worse code (i.e. because we have
only trivial parameters or no parameters at all), we might want to
investigate optimizing away the double dispatch at some point in the
future.

BUG=v8:5267,v8:5365
==========

to

==========
[turbofan] Initial support for polymorphic inlining.

For call sites where the target is not a known constant, but potentially
a list of known constants (i.e. a Phi with all HeapConstant inputs), we
do still record the call site as a potential candidate for inlining now
and in case the heuristic picks that candidate for inlining, we first
expand the call site to a dispatched call site and then invoke the
actual inlining logic for all the nested call sites.

We currently allow up to 4 targets for polymorphic inlining, just like
Crankshaft does, although we might want to refine that later.

Also note that this approach is different from what Crankshaft does in
that we don't duplicate the evaluation of the parameters per polymorphic
case, but instead we first perform the load of the target (which usually
dispatches based on the receiver map), then we evaluate all the
parameters, and then we dispatch again based on the known targets. This
might generate better or worse code compared to what Crankshaft does,
and for the cases where we generate worse code (i.e. because we have
only trivial parameters or no parameters at all), we might want to
investigate optimizing away the double dispatch at some point in the
future.

R=mvstanton@chromium.org
BUG=v8:5267,v8:5365
==========

[Benedikt Meurer, 2016-09-09 07:01:42]

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

[Benedikt Meurer, 2016-09-09 07:02:10]

Hey Michael,

Here's the polymorphic inlining.
Please take a look.

greets,
Benedikt

[mvstanton, 2016-09-09 07:40:03]

Description was changed from

==========
[turbofan] Initial support for polymorphic inlining.

For call sites where the target is not a known constant, but potentially
a list of known constants (i.e. a Phi with all HeapConstant inputs), we
do still record the call site as a potential candidate for inlining now
and in case the heuristic picks that candidate for inlining, we first
expand the call site to a dispatched call site and then invoke the
actual inlining logic for all the nested call sites.

We currently allow up to 4 targets for polymorphic inlining, just like
Crankshaft does, although we might want to refine that later.

Also note that this approach is different from what Crankshaft does in
that we don't duplicate the evaluation of the parameters per polymorphic
case, but instead we first perform the load of the target (which usually
dispatches based on the receiver map), then we evaluate all the
parameters, and then we dispatch again based on the known targets. This
might generate better or worse code compared to what Crankshaft does,
and for the cases where we generate worse code (i.e. because we have
only trivial parameters or no parameters at all), we might want to
investigate optimizing away the double dispatch at some point in the
future.

R=mvstanton@chromium.org
BUG=v8:5267,v8:5365
==========

to

==========
[turbofan] Initial support for polymorphic inlining.

For call sites where the target is not a known constant, but potentially
a list of known constants (i.e. a Phi with all HeapConstant inputs), we
still record the call site as a potential candidate for inlining.
In case the heuristic picks that candidate for inlining, we
expand the call site to a dispatched call site and invoke the
actual inlining logic for all the nested call sites.

Like Crankshaft, we currently allow up to 4 targets for polymorphic inlining,
although we might want to refine that later.

This approach is different from what Crankshaft does in
that we don't duplicate the evaluation of the parameters per polymorphic
case. Instead we first perform the load of the target (which usually
dispatches based on the receiver map), then we evaluate all the
parameters, and then we dispatch again based on the known targets. This
might generate better or worse code compared to what Crankshaft does,
and for the cases where we generate worse code (i.e. because we have
only trivial parameters or no parameters at all), we might want to
investigate optimizing away the double dispatch in the
future.

R=mvstanton@chromium.org
BUG=v8:5267,v8:5365
==========

[mvstanton, 2016-09-09 07:55:39]

LGTM with nits.

https://codereview.chromium.org/2325943002/diff/1/src/compiler/js-inlining-he...
File src/compiler/js-inlining-heuristic.cc (right):

https://codereview.chromium.org/2325943002/diff/1/src/compiler/js-inlining-he...
src/compiler/js-inlining-heuristic.cc:212: if (i != num_calls - 1) {
nit: if (i != (num_calls - 1)) {

https://codereview.chromium.org/2325943002/diff/1/src/compiler/js-inlining-he...
src/compiler/js-inlining-heuristic.cc:222: inputs[0] = target;
A comment here for the surgery, that the first input is the function to call.
and a comment for the inputs[input_count - 1] = if_successes[i] line.

[Benedikt Meurer, 2016-09-09 08:20:52]

Addressed comments, thanks.

https://codereview.chromium.org/2325943002/diff/1/src/compiler/js-inlining-he...
File src/compiler/js-inlining-heuristic.cc (right):

https://codereview.chromium.org/2325943002/diff/1/src/compiler/js-inlining-he...
src/compiler/js-inlining-heuristic.cc:212: if (i != num_calls - 1) {
On 2016/09/09 07:55:39, mvstanton wrote:
> nit: if (i != (num_calls - 1)) {

Done.

https://codereview.chromium.org/2325943002/diff/1/src/compiler/js-inlining-he...
src/compiler/js-inlining-heuristic.cc:222: inputs[0] = target;
On 2016/09/09 07:55:39, mvstanton wrote:
> A comment here for the surgery, that the first input is the function to call.
> and a comment for the inputs[input_count - 1] = if_successes[i] line.

Done.

[Benedikt Meurer, 2016-09-09 08:21:12]

The CQ bit was checked by bmeurer@chromium.org

[Benedikt Meurer, 2016-09-09 08:21:12]

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

[commit-bot: I haz the power, 2016-09-09 08:21:17]

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-09 08:44:08]

Description was changed from

==========
[turbofan] Initial support for polymorphic inlining.

For call sites where the target is not a known constant, but potentially
a list of known constants (i.e. a Phi with all HeapConstant inputs), we
still record the call site as a potential candidate for inlining.
In case the heuristic picks that candidate for inlining, we
expand the call site to a dispatched call site and invoke the
actual inlining logic for all the nested call sites.

Like Crankshaft, we currently allow up to 4 targets for polymorphic inlining,
although we might want to refine that later.

This approach is different from what Crankshaft does in
that we don't duplicate the evaluation of the parameters per polymorphic
case. Instead we first perform the load of the target (which usually
dispatches based on the receiver map), then we evaluate all the
parameters, and then we dispatch again based on the known targets. This
might generate better or worse code compared to what Crankshaft does,
and for the cases where we generate worse code (i.e. because we have
only trivial parameters or no parameters at all), we might want to
investigate optimizing away the double dispatch in the
future.

R=mvstanton@chromium.org
BUG=v8:5267,v8:5365
==========

to

==========
[turbofan] Initial support for polymorphic inlining.

For call sites where the target is not a known constant, but potentially
a list of known constants (i.e. a Phi with all HeapConstant inputs), we
still record the call site as a potential candidate for inlining.
In case the heuristic picks that candidate for inlining, we
expand the call site to a dispatched call site and invoke the
actual inlining logic for all the nested call sites.

Like Crankshaft, we currently allow up to 4 targets for polymorphic inlining,
although we might want to refine that later.

This approach is different from what Crankshaft does in
that we don't duplicate the evaluation of the parameters per polymorphic
case. Instead we first perform the load of the target (which usually
dispatches based on the receiver map), then we evaluate all the
parameters, and then we dispatch again based on the known targets. This
might generate better or worse code compared to what Crankshaft does,
and for the cases where we generate worse code (i.e. because we have
only trivial parameters or no parameters at all), we might want to
investigate optimizing away the double dispatch in the
future.

R=mvstanton@chromium.org
BUG=v8:5267,v8:5365
==========

[commit-bot: I haz the power, 2016-09-09 08:44:09]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-09 08:44:29]

Description was changed from

==========
[turbofan] Initial support for polymorphic inlining.

For call sites where the target is not a known constant, but potentially
a list of known constants (i.e. a Phi with all HeapConstant inputs), we
still record the call site as a potential candidate for inlining.
In case the heuristic picks that candidate for inlining, we
expand the call site to a dispatched call site and invoke the
actual inlining logic for all the nested call sites.

Like Crankshaft, we currently allow up to 4 targets for polymorphic inlining,
although we might want to refine that later.

This approach is different from what Crankshaft does in
that we don't duplicate the evaluation of the parameters per polymorphic
case. Instead we first perform the load of the target (which usually
dispatches based on the receiver map), then we evaluate all the
parameters, and then we dispatch again based on the known targets. This
might generate better or worse code compared to what Crankshaft does,
and for the cases where we generate worse code (i.e. because we have
only trivial parameters or no parameters at all), we might want to
investigate optimizing away the double dispatch in the
future.

R=mvstanton@chromium.org
BUG=v8:5267,v8:5365
==========

to

==========
[turbofan] Initial support for polymorphic inlining.

For call sites where the target is not a known constant, but potentially
a list of known constants (i.e. a Phi with all HeapConstant inputs), we
still record the call site as a potential candidate for inlining.
In case the heuristic picks that candidate for inlining, we
expand the call site to a dispatched call site and invoke the
actual inlining logic for all the nested call sites.

Like Crankshaft, we currently allow up to 4 targets for polymorphic inlining,
although we might want to refine that later.

This approach is different from what Crankshaft does in
that we don't duplicate the evaluation of the parameters per polymorphic
case. Instead we first perform the load of the target (which usually
dispatches based on the receiver map), then we evaluate all the
parameters, and then we dispatch again based on the known targets. This
might generate better or worse code compared to what Crankshaft does,
and for the cases where we generate worse code (i.e. because we have
only trivial parameters or no parameters at all), we might want to
investigate optimizing away the double dispatch in the
future.

R=mvstanton@chromium.org
BUG=v8:5267,v8:5365

Committed: https://crrev.com/7d4ab7d43e335f36b0f91eb36e231dc7bea048ea
Cr-Commit-Position: refs/heads/master@{#39302}
==========

[commit-bot: I haz the power, 2016-09-09 08:44:30]

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

[brucedawson, 2016-09-12 18:07:59]

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

[brucedawson, 2016-09-12 18:08:00]

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

https://codereview.chromium.org/2325943002/diff/20001/src/compiler/js-inlinin...
src/compiler/js-inlining-heuristic.cc:247:
graph()->NewNode(common()->Merge(num_calls), num_calls, if_exceptions);
Probably not a bug, but this declaration of 'control' is confusing because there
is an identically named variable in the outer scope which is used before and
after this scope. Consider renaming?

The full set of new variable shadowing warnings from the experimental VC++
/analyze (static code analysis) run for this file is:

v8\src\compiler\js-inlining-heuristic.cc(246) : warning C6246: Local declaration
of 'control' hides declaration of the same name in outer scope. For additional
information, see previous declaration at line '200' of
'c:\src\analyze_chromium\src\v8\src\compiler\js-inlining-heuristic.cc'.
v8\src\compiler\js-inlining-heuristic.cc(271) : warning C6246: Local declaration
of 'node' hides declaration of the same name in outer scope. For additional
information, see previous declaration at line '184' of
'c:\src\analyze_chromium\src\v8\src\compiler\js-inlining-heuristic.cc'.
v8\src\compiler\js-inlining-heuristic.cc(36) : warning C6246: Local declaration
of 'm' hides declaration of the same name in outer scope. For additional
information, see previous declaration at line '27' of
'c:\src\analyze_chromium\src\v8\src\compiler\js-inlining-heuristic.cc'.

Fixing is not required - do what makes sense.