Fix bug in specialization of IC-data.

runtime/vm/flow_graph_optimizer.cc

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/flow_graph_optimizer.h"
 
 #include "vm/bit_vector.h"
 #include "vm/cha.h"
 #include "vm/dart_entry.h"
 #include "vm/flow_graph_builder.h"
@@ skipping 168 matching lines @@
 
 
 static const ICData& SpecializeICData(const ICData& ic_data, intptr_t cid) {
   ASSERT(ic_data.num_args_tested() == 1);
 
   if ((ic_data.NumberOfChecks() == 1) &&
       (ic_data.GetReceiverClassIdAt(0) == cid)) {
     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.

 }
 
 
 void FlowGraphOptimizer::SpecializePolymorphicInstanceCall(
     PolymorphicInstanceCallInstr* call) {
   if (!call->with_checks()) {
     return;  // Already specialized.
   }
 
   const intptr_t receiver_cid =
       call->PushArgumentAt(0)->value()->Type()->ToCid();
   if (receiver_cid == kDynamicCid) {
     return;  // No information about receiver was infered.
   }
 
   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());
+  }
 }
 
 
 static BinarySmiOpInstr* AsSmiShiftLeftInstruction(Definition* d) {
   BinarySmiOpInstr* instr = d->AsBinarySmiOp();
   if ((instr != NULL) && (instr->op_kind() == Token::kSHL)) {
     return instr;
   }
   return NULL;
 }
@@ skipping 8078 matching lines @@
   }
 
   // Insert materializations at environment uses.
   for (intptr_t i = 0; i < exits.length(); i++) {
     CreateMaterializationAt(exits[i], alloc, alloc->cls(), *fields);
   }
 }
 
 
 }  // namespace dart