Fix bug in specialization of IC-data.

Fix bug in specialization of IC-data.

My previous change to use constants for OSR values if possible
uncovered a bug when narrowing IC-data based on type information:

If the exising IC data don't agree with the static type information,
don't specialize.

R=vegorov@google.com

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

[Vyacheslav Egorov (Google), 2014-01-08 13:00:00]

lgtm

https://codereview.chromium.org/128113003/diff/1/runtime/vm/flow_graph_optimi...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/128113003/diff/1/runtime/vm/flow_graph_optimi...
runtime/vm/flow_graph_optimizer.cc:192: const ICData& new_ic_data =
ICData::ZoneHandle(ICData::New(
You can actually look up target function on the class if it is not available in
the ICData. That would allow to specialize the not yet executed code paths
statically.

https://codereview.chromium.org/128113003/diff/1/runtime/vm/flow_graph_optimi...
runtime/vm/flow_graph_optimizer.cc:202: return ic_data;
add empty line before return for readability.

https://codereview.chromium.org/128113003/diff/1/runtime/vm/flow_graph_optimi...
runtime/vm/flow_graph_optimizer.cc:218: const ICData& ic_data =
SpecializeICData(call->ic_data(), receiver_cid);
I would rename SpecializeICData to TrySpecializeICData now to signal that it can
fail and that one needs to check if it actually succeeded.

https://codereview.chromium.org/128113003/diff/1/runtime/vm/flow_graph_optimi...
runtime/vm/flow_graph_optimizer.cc:219: if (ic_data.raw() !=
call->ic_data().raw()) {
You can change this to match the early exit pattern used in the rest of this
function

if (ic_data.raw() == call->ic_data().raw()) {
  // Specialization failed.
  return;
}

const bool with_check = ...

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

https://codereview.chromium.org/128113003/diff/1/runtime/vm/intermediate_lang...
runtime/vm/intermediate_language.cc:2019: if (!ic_data().HasOneTarget()) return
false;
Accidental change? Please revert.

[Florian Schneider, 2014-01-08 13:07:57]

Committed patchset #2 manually as r31607 (presubmit successful).

[Florian Schneider, 2014-01-08 14:12:42]

https://codereview.chromium.org/128113003/diff/1/runtime/vm/flow_graph_optimi...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/128113003/diff/1/runtime/vm/flow_graph_optimi...
runtime/vm/flow_graph_optimizer.cc:192: const ICData& new_ic_data =
ICData::ZoneHandle(ICData::New(
On 2014/01/08 13:00:00, Vyacheslav Egorov (Google) wrote:
> You can actually look up target function on the class if it is not available
in
> the ICData. That would allow to specialize the not yet executed code paths
> statically.

Added a TODO.

https://codereview.chromium.org/128113003/diff/1/runtime/vm/flow_graph_optimi...
runtime/vm/flow_graph_optimizer.cc:202: return ic_data;
On 2014/01/08 13:00:00, Vyacheslav Egorov (Google) wrote:
> add empty line before return for readability.

Done.

https://codereview.chromium.org/128113003/diff/1/runtime/vm/flow_graph_optimi...
runtime/vm/flow_graph_optimizer.cc:218: const ICData& ic_data =
SpecializeICData(call->ic_data(), receiver_cid);
On 2014/01/08 13:00:00, Vyacheslav Egorov (Google) wrote:
> I would rename SpecializeICData to TrySpecializeICData now to signal that it
can
> fail and that one needs to check if it actually succeeded.

Done.

https://codereview.chromium.org/128113003/diff/1/runtime/vm/flow_graph_optimi...
runtime/vm/flow_graph_optimizer.cc:219: if (ic_data.raw() !=
call->ic_data().raw()) {
On 2014/01/08 13:00:00, Vyacheslav Egorov (Google) wrote:
> You can change this to match the early exit pattern used in the rest of this
> function
> 
> if (ic_data.raw() == call->ic_data().raw()) {
>   // Specialization failed.
>   return;
> }
> 
> const bool with_check = ...

Done.

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

https://codereview.chromium.org/128113003/diff/1/runtime/vm/intermediate_lang...
runtime/vm/intermediate_language.cc:2019: if (!ic_data().HasOneTarget()) return
false;
On 2014/01/08 13:00:00, Vyacheslav Egorov (Google) wrote:
> Accidental change? Please revert.

Done.

[srdjan, 2014-01-08 17:31:47]

lgtm