Fix bug in specialization of IC-data.

runtime/vm/flow_graph_optimizer.cc

Index: runtime/vm/flow_graph_optimizer.cc
===================================================================
--- runtime/vm/flow_graph_optimizer.cc	(revision 31601)
+++ runtime/vm/flow_graph_optimizer.cc	(working copy)
@@ -186,21 +186,20 @@
     return ic_data;  // Nothing to do
   }
 
-  const ICData& new_ic_data = ICData::ZoneHandle(ICData::New(
-      Function::Handle(ic_data.function()),
-      String::Handle(ic_data.target_name()),
-      Object::empty_array(),  // Dummy argument descriptor.
-      ic_data.deopt_id(),
-      ic_data.num_args_tested()));
-  new_ic_data.set_deopt_reason(ic_data.deopt_reason());
-
   const Function& function =
       Function::Handle(ic_data.GetTargetForReceiverClassId(cid));
   if (!function.IsNull()) {
+    const ICData& new_ic_data = ICData::ZoneHandle(ICData::New(

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

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.

[Florian Schneider, 2014/01/08 14:12:43]

Added a TODO.

+        Function::Handle(ic_data.function()),
+        String::Handle(ic_data.target_name()),
+        Object::empty_array(),  // Dummy argument descriptor.
+        ic_data.deopt_id(),
+        ic_data.num_args_tested()));
+    new_ic_data.set_deopt_reason(ic_data.deopt_reason());
     new_ic_data.AddReceiverCheck(cid, function);
+    return new_ic_data;
   }
-
-  return new_ic_data;
+  return ic_data;

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

add empty line before return for readability.

[Florian Schneider, 2014/01/08 14:12:43]

Done.

 }
 
 
@@ -217,13 +216,15 @@
   }
 
   const ICData& ic_data = SpecializeICData(call->ic_data(), receiver_cid);

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

I would rename SpecializeICData to TrySpecializeICData now to signal that it can
fail and that one needs to check if it actually succeeded.

[Florian Schneider, 2014/01/08 14:12:43]

Done.

-
-  const bool with_checks = false;
-  PolymorphicInstanceCallInstr* specialized =
-      new PolymorphicInstanceCallInstr(call->instance_call(),
-                                       ic_data,
-                                       with_checks);
-  call->ReplaceWith(specialized, current_iterator());
+  if (ic_data.raw() != call->ic_data().raw()) {

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

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 = ...

[Florian Schneider, 2014/01/08 14:12:43]

Done.

+    // Only replace the call if ICData were specialized.
+    const bool with_checks = false;
+    PolymorphicInstanceCallInstr* specialized =
+        new PolymorphicInstanceCallInstr(call->instance_call(),
+                                         ic_data,
+                                         with_checks);
+    call->ReplaceWith(specialized, current_iterator());
+  }
 }