Some cleanups, started implementing checked instance calls, better equality operation.

runtime/vm/flow_graph_optimizer.cc

Index: runtime/vm/flow_graph_optimizer.cc
===================================================================
--- runtime/vm/flow_graph_optimizer.cc	(revision 8588)
+++ runtime/vm/flow_graph_optimizer.cc	(working copy)
@@ -158,11 +158,11 @@
 }
 
 
-void FlowGraphOptimizer::TryReplaceWithBinaryOp(InstanceCallComp* comp,
+bool FlowGraphOptimizer::TryReplaceWithBinaryOp(InstanceCallComp* comp,
                                                 Token::Kind op_kind) {
   if (comp->ic_data()->NumberOfChecks() != 1) {
     // TODO(srdjan): Not yet supported.
-    return;
+    return false;
   }
 
   BinaryOpComp::OperandsType operands_type;
@@ -171,7 +171,7 @@
     if (op_kind == Token::kDIV ||
         op_kind == Token::kMOD) {
       // TODO(srdjan): Not yet supported.
-      return;
+      return false;
     }
 
     operands_type = BinaryOpComp::kSmiOperands;
@@ -181,13 +181,13 @@
         op_kind != Token::kMUL &&
         op_kind != Token::kDIV) {
       // TODO(vegorov): Not yet supported.
-      return;
+      return false;
     }
 
     operands_type = BinaryOpComp::kDoubleOperands;
   } else {
     // TODO(srdjan): Not yet supported.
-    return;
+    return false;
   }
 
   ASSERT(comp->instr() != NULL);
@@ -200,18 +200,17 @@
                        comp,
                        left,
                        right);
-  ASSERT(bin_op->ic_data() == NULL);
   bin_op->set_ic_data(comp->ic_data());
-  bin_op->set_instr(comp->instr());
-  comp->instr()->replace_computation(bin_op);
+  comp->ReplaceWith(bin_op);
+  return true;
 }
 
 
-void FlowGraphOptimizer::TryReplaceWithUnaryOp(InstanceCallComp* comp,
-                                              Token::Kind op_kind) {
+bool FlowGraphOptimizer::TryReplaceWithUnaryOp(InstanceCallComp* comp,
+                                               Token::Kind op_kind) {
   if (comp->ic_data()->NumberOfChecks() != 1) {
     // TODO(srdjan): Not yet supported.
-    return;
+    return false;
   }
   ASSERT(comp->instr() != NULL);
   ASSERT(comp->InputCount() == 1);
@@ -222,11 +221,11 @@
     unary_op = new NumberNegateComp(comp, comp->InputAt(0));
   }
   if (unary_op != NULL) {
-    ASSERT(unary_op->ic_data() == NULL);
     unary_op->set_ic_data(comp->ic_data());
-    unary_op->set_instr(comp->instr());
-    comp->instr()->replace_computation(unary_op);
+    comp->ReplaceWith(unary_op);
+    return true;
   }
+  return false;
 }
 
 
@@ -266,40 +265,75 @@
 }
 
 
-// Returns array of all class ids that are in ic_data. The result is
-// normalized so that a smi class is at index 0 if it exists in the ic_data.
-static ZoneGrowableArray<intptr_t>*  ExtractClassIds(const ICData& ic_data) {
-  if (ic_data.NumberOfChecks() == 0) return NULL;
-  ZoneGrowableArray<intptr_t>* result =
-      new ZoneGrowableArray<intptr_t>(ic_data.NumberOfChecks());
+// Returns all receiver class-ids and corresponding tagets for the given
+// 'ic_data', sorted so that a smi class id is at index[0] if it exists.
+static void ExtractClassIdsAndTargets(const ICData& ic_data,
+                                      ZoneGrowableArray<intptr_t>* class_ids,
+                                      ZoneGrowableArray<Function*>* targets) {
+  ASSERT(class_ids != NULL);
+  ASSERT(targets != NULL);
+  class_ids->Clear();
+  targets->Clear();
   intptr_t smi_index = -1;
   Function& target = Function::Handle();
-  Class& cls = Class::Handle();
+  GrowableArray<const Class*> classes;
   for (intptr_t i = 0; i < ic_data.NumberOfChecks(); i++) {
-    ic_data.GetOneClassCheckAt(i, &cls, &target);
-    result->Add(cls.id());
-    if (cls.id() == kSmi) {
+    ic_data.GetCheckAt(i, &classes, &target);
+    // Collect receiver class only.
+    const intptr_t class_id = (*classes[0]).id();
+    if (ic_data.num_args_tested() > 1) {
+      // Check if we have not already entered the class-id.
+      Function* target_found = NULL;
+      for (intptr_t k = 0; k < class_ids->length(); k++) {
+        if ((*class_ids)[k] == class_id) {
+          target_found = (*targets)[k];
+          break;
+        }
+      }
+      if (target_found != NULL) {
+        ASSERT(target_found->raw() == target.raw());
+        continue;
+      }
+    }
+    if (class_id == kSmi) {
       ASSERT(smi_index < 0);  // Classes entered only once in ic_data.
-      smi_index = i;
+      smi_index = class_ids->length();
     }
+    class_ids->Add(class_id);
+    targets->Add(&Function::ZoneHandle(target.raw()));
   }
   if (smi_index >= 0) {
     // Smi class id must be at index 0.
-    intptr_t temp = (*result)[0];
-    (*result)[0] =  (*result)[smi_index];
-    (*result)[smi_index] = temp;
+    intptr_t temp_id = (*class_ids)[0];
+    Function* temp_func = (*targets)[0];
+    (*class_ids)[0] =  (*class_ids)[smi_index];
+    (*targets)[0] =  (*targets)[smi_index];
+    (*class_ids)[smi_index] = temp_id;
+    (*targets)[smi_index] = temp_func;
   }
+}
+
+
+// Returns array of all class ids that are in ic_data. The result is
+// normalized so that a smi class is at index 0 if it exists in the ic_data.
+static ZoneGrowableArray<intptr_t>*  ExtractClassIds(const ICData& ic_data) {
+  if (ic_data.NumberOfChecks() == 0) return NULL;
+  ZoneGrowableArray<intptr_t>* result =
+      new ZoneGrowableArray<intptr_t>(ic_data.NumberOfChecks());
+  ZoneGrowableArray<Function*>* dummy =
+      new ZoneGrowableArray<Function*>(ic_data.NumberOfChecks());
+  ExtractClassIdsAndTargets(ic_data, result, dummy);

[Vyacheslav Egorov (Google), 2012/06/13 09:02:27]

how about passing NULL instead of dummy? Collecting stuff that is not used is a
bit wasteful.

[srdjan, 2012/06/13 18:34:24]

Done.

   return result;
 }
 
 
 // Only unique implicit instance getters can be currently handled.
-void FlowGraphOptimizer::TryInlineInstanceGetter(InstanceCallComp* comp) {
+bool FlowGraphOptimizer::TryInlineInstanceGetter(InstanceCallComp* comp) {
   ASSERT(comp->HasICData());
   const ICData& ic_data = *comp->ic_data();
   if (ic_data.NumberOfChecks() == 0) {
     // No type feedback collected.
-    return;
+    return false;
   }
   Function& target = Function::Handle();
   GrowableArray<const Class*> classes;
@@ -309,7 +343,7 @@
   if (target.kind() == RawFunction::kImplicitGetter) {
     if (!HasOneTarget(ic_data)) {
       // TODO(srdjan): Implement for mutiple targets.
-      return;
+      return false;
     }
     // Inline implicit instance getter.
     const String& field_name =
@@ -318,10 +352,8 @@
     ASSERT(!field.IsNull());
     LoadInstanceFieldComp* load = new LoadInstanceFieldComp(
         field, comp->InputAt(0), comp, ExtractClassIds(ic_data));
-    // Replace 'comp' with 'load'.
-    load->set_instr(comp->instr());
-    comp->instr()->replace_computation(load);
-    return;
+    comp->ReplaceWith(load);
+    return true;
   }
 
   // Not an implicit getter.
@@ -334,7 +366,7 @@
       (recognized_kind == MethodRecognizer::kGrowableArrayLength)) {
     if (!HasOneTarget(ic_data)) {
       // TODO(srdjan): Implement for mutiple targets.
-      return;
+      return false;
     }
     intptr_t length_offset = -1;
     switch (recognized_kind) {
@@ -354,9 +386,8 @@
         Type::ZoneHandle(Type::IntInterface()),
         comp,
         ExtractClassIds(ic_data));
-    load->set_instr(comp->instr());
-    comp->instr()->replace_computation(load);
-    return;
+    comp->ReplaceWith(load);
+    return true;
   }
 
   if (recognized_kind == MethodRecognizer::kStringBaseLength) {
@@ -367,25 +398,24 @@
         Type::ZoneHandle(Type::IntInterface()),
         comp,
         ExtractClassIds(ic_data));
-    load->set_instr(comp->instr());
-    comp->instr()->replace_computation(load);
-    return;
+    comp->ReplaceWith(load);
+    return true;
   }
+  return false;
 }
 
 
 // Inline only simple, frequently called core library methods.
-void FlowGraphOptimizer::TryInlineInstanceMethod(InstanceCallComp* comp) {
+bool FlowGraphOptimizer::TryInlineInstanceMethod(InstanceCallComp* comp) {
   ASSERT(comp->HasICData());
   const ICData& ic_data = *comp->ic_data();
   if ((ic_data.NumberOfChecks() == 0) || !HasOneTarget(ic_data)) {
     // No type feedback collected.
-    return;
+    return false;
   }
   Function& target = Function::Handle();
   GrowableArray<const Class*> classes;
   ic_data.GetCheckAt(0, &classes, &target);
-  ASSERT(classes.length() == 1);
   MethodRecognizer::Kind recognized_kind =
       MethodRecognizer::RecognizeKind(target);
   if (recognized_kind == MethodRecognizer::kDoubleToDouble) {
@@ -394,27 +424,38 @@
   if (recognized_kind == MethodRecognizer::kIntegerToDouble) {
     // TODO(srdjan): Implement.
   }
+  return false;
 }
 
 
 void FlowGraphOptimizer::VisitInstanceCall(InstanceCallComp* comp) {
-  if (comp->HasICData()) {
+  if (comp->HasICData() && (comp->ic_data()->NumberOfChecks() > 0)) {
     const String& function_name = comp->function_name();
     Token::Kind op_kind = Token::GetBinaryOp(function_name);

[Vyacheslav Egorov (Google), 2012/06/13 09:02:27]

Can we just store op_kind in the InstanceCallComp produced from binary/unary
ops? Parsing it seems so strange when builder actually knew what kind of token
was there.

The same for getter (we can mark them with kGet token)

[srdjan, 2012/06/13 18:34:24]

Yes, next CL.

-    if (op_kind != Token::kILLEGAL) {
-      TryReplaceWithBinaryOp(comp, op_kind);
+    if ((op_kind != Token::kILLEGAL) && TryReplaceWithBinaryOp(comp, op_kind)) {
       return;
     }
     op_kind = Token::GetUnaryOp(function_name);
-    if (op_kind != Token::kILLEGAL) {
-      TryReplaceWithUnaryOp(comp, op_kind);
+    if ((op_kind != Token::kILLEGAL) && TryReplaceWithUnaryOp(comp, op_kind)) {
       return;
     }
-    if (Field::IsGetterName(function_name)) {
-      TryInlineInstanceGetter(comp);
+    if ((Field::IsGetterName(function_name)) && TryInlineInstanceGetter(comp)) {
       return;
     }
-    TryInlineInstanceMethod(comp);
+    if (TryInlineInstanceMethod(comp)) {
+      return;
+    }
+    const intptr_t kMaxChecks = 4;
+    if (comp->ic_data()->num_args_tested() <= kMaxChecks) {
+      ZoneGrowableArray<intptr_t>* class_ids =
+          new ZoneGrowableArray<intptr_t>();
+      ZoneGrowableArray<Function*>* targets =
+          new ZoneGrowableArray<Function*>();
+      ExtractClassIdsAndTargets(*comp->ic_data(), class_ids, targets);
+      CheckedInstanceCallComp* call =
+          new CheckedInstanceCallComp(comp, *class_ids, *targets);
+      comp->ReplaceWith(call);
+    }
   }
 }
 
@@ -428,16 +469,16 @@
 }
 
 
-void FlowGraphOptimizer::TryInlineInstanceSetter(InstanceSetterComp* comp) {
+bool FlowGraphOptimizer::TryInlineInstanceSetter(InstanceSetterComp* comp) {
   ASSERT(comp->HasICData());
   const ICData& ic_data = *comp->ic_data();
   if (ic_data.NumberOfChecks() == 0) {
     // No type feedback collected.
-    return;
+    return false;
   }
   if (!HasOneTarget(ic_data)) {
     // TODO(srdjan): Implement when not all targets are the sa,e.
-    return;
+    return false;
   }
   Function& target = Function::Handle();
   Class& cls = Class::Handle();
@@ -445,7 +486,7 @@
   if (target.kind() != RawFunction::kImplicitSetter) {
     // Not an implicit setter.
     // TODO(srdjan): Inline special setters.
-    return;
+    return false;
   }
   // Inline implicit instance setter.
   const Field& field = Field::Handle(GetField(cls, comp->field_name()));
@@ -456,9 +497,8 @@
       comp->InputAt(1),
       comp,
       ExtractClassIds(ic_data));
-  // Replace 'comp' with 'store'.
-  store->set_instr(comp->instr());
-  comp->instr()->replace_computation(store);
+  comp->ReplaceWith(store);
+  return true;
 }
 
 
@@ -510,6 +550,17 @@
 }
 
 
+void FlowGraphOptimizer::VisitEqualityCompare(EqualityCompareComp* comp) {
+  if (!comp->HasICData()) return;
+  const ICData& ic_data = *comp->ic_data();
+  if (ic_data.NumberOfChecks() != 1) return;
+  ASSERT(HasOneTarget(ic_data));
+  if (HasTwoSmi(ic_data)) {
+    comp->set_operands_class_id(kSmi);
+  }
+}
+
+
 void FlowGraphOptimizer::VisitDo(DoInstr* instr) {
   instr->computation()->Accept(this);
 }