Fix polymorphic inlining of method dispatchers.

Fix polymorphic inlining of method dispatchers.

Field dispatchers and noSuchMethod dispatchers are generated by the VM
compiler and should always be inlined - this CL fixes the case of polymorphic
inlining where dipatchers were not inlined in the same way as for monomorphic
calls sites.

R=iposva@google.com, srdjan@google.com

Committed: https://code.google.com/p/dart/source/detail?r=35490

[Florian Schneider, 2014-04-11 11:34:32]

This fixes part of the performance regression after my change to closure calls:
Polymorphic sites of dispatcher calls are now inlined in the same way as
monomorphic sites.

There is still the problem that the closure call-sites inside library functions
(forEach) become polymorphic with different signature classes. For this we need
to specialize IC data inside forEach with the call-site specific IC data - a
general problem that we don't have per-site specialization when inlining
polymorphic call sites.

[srdjan, 2014-04-11 17:01:04]

lgtm

https://codereview.chromium.org/235013002/diff/1/runtime/vm/intermediate_lang...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/235013002/diff/1/runtime/vm/intermediate_lang...
runtime/vm/intermediate_language.cc:2178: const Function& target =
Function::Handle(ic_data().GetTargetAt(i));
'Allocate' handle outside the loop.

[Ivan Posva, 2014-04-11 21:12:00]

I am retracting Srdjan's LGTM.

The way I understand this code it has holes that would allow for really subtle
bugs with field dispatchers from different base classes being mixed.

-Ivan

https://codereview.chromium.org/235013002/diff/1/runtime/vm/flow_graph_inline...
File runtime/vm/flow_graph_inliner.cc (right):

https://codereview.chromium.org/235013002/diff/1/runtime/vm/flow_graph_inline...
runtime/vm/flow_graph_inliner.cc:305: instance_call->HasOnlyDispatcherTargets())
{
This is not correct. Only checking for dispatcher targets is too permissive.

[Florian Schneider, 2014-04-22 00:09:10]

https://codereview.chromium.org/235013002/diff/1/runtime/vm/flow_graph_inline...
File runtime/vm/flow_graph_inliner.cc (right):

https://codereview.chromium.org/235013002/diff/1/runtime/vm/flow_graph_inline...
runtime/vm/flow_graph_inliner.cc:305: instance_call->HasOnlyDispatcherTargets())
{
On 2014/04/11 21:12:01, Ivan Posva wrote:
> This is not correct. Only checking for dispatcher targets is too permissive.

Permissive for what?

[Ivan Posva, 2014-04-24 07:28:47]

https://codereview.chromium.org/235013002/diff/1/runtime/vm/flow_graph_inline...
File runtime/vm/flow_graph_inliner.cc (right):

https://codereview.chromium.org/235013002/diff/1/runtime/vm/flow_graph_inline...
runtime/vm/flow_graph_inliner.cc:305: instance_call->HasOnlyDispatcherTargets())
{
On 2014/04/22 00:09:10, Florian Schneider wrote:
> On 2014/04/11 21:12:01, Ivan Posva wrote:
> > This is not correct. Only checking for dispatcher targets is too permissive.
> 
> Permissive for what?

There is not necessarily only a single recognized target only because this
instance call has only dispatcher targets.

[Florian Schneider, 2014-04-24 12:00:44]

https://codereview.chromium.org/235013002/diff/1/runtime/vm/flow_graph_inline...
File runtime/vm/flow_graph_inliner.cc (right):

https://codereview.chromium.org/235013002/diff/1/runtime/vm/flow_graph_inline...
runtime/vm/flow_graph_inliner.cc:305: instance_call->HasOnlyDispatcherTargets())
{
On 2014/04/24 07:28:47, Ivan Posva wrote:
> On 2014/04/22 00:09:10, Florian Schneider wrote:
> > On 2014/04/11 21:12:01, Ivan Posva wrote:
> > > This is not correct. Only checking for dispatcher targets is too
permissive.
> > 
> > Permissive for what?
> 
> There is not necessarily only a single recognized target only because this
> instance call has only dispatcher targets.

There is no correctness requirement that the call site has a single recognized
target. This condition could be widened to multiple recognized targets.

The condition is here to inline recognized targets when we at the deepest
allowed inlining depth.

I added the case that dispatchers are always inlined at the max inlining depth.

[Florian Schneider, 2014-04-28 18:06:51]

Committed patchset #1 manually as r35490 (presubmit successful).