Noopt work: populate empty ICData of getters and setters with function's own class if that getter/s…

runtime/vm/flow_graph_optimizer.cc

Index: runtime/vm/flow_graph_optimizer.cc
diff --git a/runtime/vm/flow_graph_optimizer.cc b/runtime/vm/flow_graph_optimizer.cc
index 91e6a58dbb6370afc112530f318645444b1c48cc..d6ff370ab14d21590ec52359460bfb1279c7309b 100644
--- a/runtime/vm/flow_graph_optimizer.cc
+++ b/runtime/vm/flow_graph_optimizer.cc
@@ -45,6 +45,8 @@ DEFINE_FLAG(bool, use_cha_deopt, true,
 #if defined(TARGET_ARCH_ARM) || defined(TARGET_ARCH_IA32)
 DEFINE_FLAG(bool, trace_smi_widening, false, "Trace Smi->Int32 widening pass.");
 #endif
+
+DECLARE_FLAG(bool, polymorphic_with_deopt);
 DECLARE_FLAG(bool, source_lines);
 DECLARE_FLAG(bool, trace_type_check_elimination);
 DECLARE_FLAG(bool, warn_on_javascript_compatibility);
@@ -202,44 +204,65 @@ bool FlowGraphOptimizer::TryCreateICData(InstanceCallInstr* call) {
     }
   }
 
+  bool all_cids_known = true;
   for (intptr_t i = 0; i < class_ids.length(); i++) {
     if (class_ids[i] == kDynamicCid) {
       // Not all cid-s known.
-      return false;
+      all_cids_known = false;
+      break;
     }
   }
 
-  const Array& args_desc_array = Array::Handle(Z,
-      ArgumentsDescriptor::New(call->ArgumentCount(), call->argument_names()));
-  ArgumentsDescriptor args_desc(args_desc_array);
-  const Class& receiver_class = Class::Handle(Z,
-      isolate()->class_table()->At(class_ids[0]));
-  const Function& function = Function::Handle(Z,
-      Resolver::ResolveDynamicForReceiverClass(
-          receiver_class,
-          call->function_name(),
-          args_desc));
-  if (function.IsNull()) {
-    return false;
+  if (all_cids_known) {
+    const Array& args_desc_array = Array::Handle(Z,
+        ArgumentsDescriptor::New(call->ArgumentCount(),
+                                 call->argument_names()));
+    ArgumentsDescriptor args_desc(args_desc_array);
+    const Class& receiver_class = Class::Handle(Z,
+        isolate()->class_table()->At(class_ids[0]));
+    const Function& function = Function::Handle(Z,
+        Resolver::ResolveDynamicForReceiverClass(
+            receiver_class,
+            call->function_name(),
+            args_desc));
+    if (function.IsNull()) {
+      return false;
+    }
+    if (class_ids.length() > 1) {
+      call->ic_data()->AddCheck(class_ids, function);
+    } else {
+      ASSERT(class_ids.length() == 1);
+      call->ic_data()->AddReceiverCheck(class_ids[0], function);
+    }
+    return true;
   }
-  // Create new ICData, do not modify the one attached to the instruction
-  // since it is attached to the assembly instruction itself.
-  // TODO(srdjan): Prevent modification of ICData object that is
-  // referenced in assembly code.
-  ICData& ic_data = ICData::ZoneHandle(Z, ICData::New(
-      flow_graph_->function(),
-      call->function_name(),
-      args_desc_array,
-      call->deopt_id(),
-      class_ids.length()));
-  if (class_ids.length() > 1) {
-    ic_data.AddCheck(class_ids, function);
-  } else {
-    ASSERT(class_ids.length() == 1);
-    ic_data.AddReceiverCheck(class_ids[0], function);
+  return false;

[Florian Schneider, 2015/06/18 15:51:10]

This return makes the code below unreachable.

Make sure the code below is reachable and also tested.

[srdjan, 2015/06/18 16:16:06]

Oops, uploaded a cl with the performance testing experiment. All tests are run
without this 'return false'.

[Florian Schneider, 2015/06/18 17:34:36]

Ok.

+
+  // Check if getter or setter
+  if ((call->token_kind() == Token::kGET) ||
+      (call->token_kind() == Token::kSET)) {
+    const Class& owner_class = Class::Handle(Z, function().Owner());
+    if (!owner_class.is_abstract()) {
+      // Quite aggressive: if functions's owner has a a getter/setter of that
+      // name we add a check and call the setter directly. Considerable
+      // performance improvement, some increase in code space.
+      // TODO(srdjan): Make sure the getter/setters can be inlined.
+      const Array& args_desc_array = Array::Handle(Z,
+          ArgumentsDescriptor::New(call->ArgumentCount(),
+                                   call->argument_names()));
+      ArgumentsDescriptor args_desc(args_desc_array);
+      const Function& function = Function::Handle(Z,
+          Resolver::ResolveDynamicForReceiverClass(owner_class,
+                                                   call->function_name(),
+                                                   args_desc));
+      if (!function.IsNull()) {
+        call->ic_data()->AddReceiverCheck(owner_class.id(), function);
+        return true;
+      }
+    }
   }
-  call->set_ic_data(&ic_data);
-  return true;
+
+  return false;
 }
 
 
@@ -273,6 +296,10 @@ const ICData& FlowGraphOptimizer::TrySpecializeICData(const ICData& ic_data,
 
 void FlowGraphOptimizer::SpecializePolymorphicInstanceCall(
     PolymorphicInstanceCallInstr* call) {
+  if (!FLAG_polymorphic_with_deopt) {
+    // Specialization adds receiver checks which can lead to deoptimization.
+    return;
+  }
   if (!call->with_checks()) {
     return;  // Already specialized.
   }
@@ -4221,9 +4248,14 @@ void FlowGraphOptimizer::InstanceCallNoopt(InstanceCallInstr* instr) {
   // deoptimize.
   const Token::Kind op_kind = instr->token_kind();
   if (instr->HasICData() && (instr->ic_data()->NumberOfUsedChecks() > 0)) {
-    if ((op_kind == Token::kGET) && TryInlineInstanceGetter(instr, false)) {
-      return;
-    }
+    const ICData& unary_checks =
+        ICData::ZoneHandle(Z, instr->ic_data()->AsUnaryClassChecks());
+
+    PolymorphicInstanceCallInstr* call =
+        new(Z) PolymorphicInstanceCallInstr(instr, unary_checks,
+                                            true /* call_with_checks*/);
+    instr->ReplaceWith(call, current_iterator());
+    return;
   }
 
   // Type test is special as it always gets converted into inlined code.
@@ -4336,7 +4368,7 @@ void FlowGraphOptimizer::VisitInstanceCall(InstanceCallInstr* instr) {
 
   if (unary_checks.NumberOfChecks() <= FLAG_max_polymorphic_checks) {
     bool call_with_checks;
-    if (has_one_target) {
+    if (has_one_target && FLAG_polymorphic_with_deopt) {
       // Type propagation has not run yet, we cannot eliminate the check.
       AddReceiverCheck(instr);
       // Call can still deoptimize, do not detach environment from instr.