Convert EqualityCompare to StrictCompare if reciever is either null or guaranteed to have default e…

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 d472575b40e1fe628e6edecdcded5cf7c0202320..187098087bd60a6a8218a2a3237b994dc2bd41fd 100644
--- a/runtime/vm/flow_graph_optimizer.cc
+++ b/runtime/vm/flow_graph_optimizer.cc
@@ -70,6 +70,9 @@ void FlowGraphOptimizer::ApplyClassIds() {
         ComparisonInstr* compare = instr->AsBranch()->comparison();
         if (compare->IsStrictCompare()) {
           VisitStrictCompare(compare->AsStrictCompare());
+        } else if (compare->IsEqualityCompare()) {
+          StrictifyEqualityCompare(compare->AsEqualityCompare(),
+                                   instr->AsBranch());
         }
       }
     }
@@ -2184,24 +2187,73 @@ void FlowGraphOptimizer::VisitRelationalOp(RelationalOpInstr* instr) {
 }
 
 
+bool FlowGraphOptimizer::CanStrictifyEqualityCompare(

[srdjan, 2013/04/18 16:54:49]

Can this be a const function?

[Vyacheslav Egorov (Google), 2013/04/18 19:48:36]

Made it static.

+    EqualityCompareInstr* compare) {
+  // If one of the inputs is null this is a strict comparison.
+  if (compare->left()->BindsToConstantNull() ||
+      compare->right()->BindsToConstantNull()) {
+    return true;
+  }
+
+  if (compare->left()->Type()->IsNone()) {
+    return false;  // We might be running prior to any type propagation passes.
+  }
+
+  // Try resolving target function using propagated cid for the receiver.
+  // If receiver is either null or has default equality operator then
+  // we can convert such comparison to a strict one.
+  const intptr_t receiver_cid =
+     compare->left()->Type()->ToNullableCid();
+
+  if (receiver_cid == kDynamicCid) {
+    return false;
+  }
+
+  const Class& receiver_class = Class::Handle(
+      Isolate::Current()->class_table()->At(receiver_cid));
+  const Function& function = Function::Handle(
+      Resolver::ResolveDynamicForReceiverClass(
+          receiver_class,
+          Symbols::EqualOperator(),
+          2,
+          0));

[srdjan, 2013/04/18 16:54:49]

Don't you need to look up the super chain if any (but not Object) class
implements equality operator?

[srdjan, 2013/04/18 17:28:59]

Correction: ResolveDynamicForReceiverClass goes up the chain. I wonder how does
it work since there is always going to be at least the Object's equality
operator function as result.

[Vyacheslav Egorov (Google), 2013/04/18 19:48:36]

Object has kInstanceCid cid that is how it works. Though it is quite confusing.

+
+  if (function.IsNull()) {
+    return false;
+  }
+
+  return (Class::Handle(function.Owner()).id() == kInstanceCid);
+}
+
+
 template <typename T>
-void FlowGraphOptimizer::HandleEqualityCompare(EqualityCompareInstr* comp,
-                                               T current_instruction) {
-  // If one of the inputs is null, no ICdata will be collected.
-  if (comp->left()->BindsToConstantNull() ||
-      comp->right()->BindsToConstantNull()) {
-    Token::Kind strict_kind = (comp->kind() == Token::kEQ) ?
+bool FlowGraphOptimizer::StrictifyEqualityCompare(EqualityCompareInstr* compare,
+                                                  T current_instruction) {

[srdjan, 2013/04/18 16:54:49]

Can this be a const function?

[srdjan, 2013/04/18 16:54:49]

I assume that it is templated bcause current_instruction could be either
Definition or ControlInstruction?

[Vyacheslav Egorov (Google), 2013/04/18 19:48:36]

Made it const.

[Vyacheslav Egorov (Google), 2013/04/18 19:48:36]

Yes, for the same reason the HandleEqualityCompare is templated.

I think we should really refactor our handling of comparisons: make them always
hosted either inside of the BranchInstr or IfThenElseInstr.

I'll keep that in the list of things to do. 

+  if (CanStrictifyEqualityCompare(compare)) {
+    Token::Kind strict_kind = (compare->kind() == Token::kEQ) ?
         Token::kEQ_STRICT : Token::kNE_STRICT;
     StrictCompareInstr* strict_comp =
         new StrictCompareInstr(strict_kind,
-                               comp->left()->Copy(),
-                               comp->right()->Copy());
+                               compare->left()->CopyWithType(),
+                               compare->right()->CopyWithType());
     current_instruction->ReplaceWith(strict_comp, current_iterator());
+    return true;
+  }
+  return false;
+}
+
+
+template <typename T>
+void FlowGraphOptimizer::HandleEqualityCompare(EqualityCompareInstr* comp,
+                                               T current_instruction) {
+  if (StrictifyEqualityCompare(comp, current_instruction)) {
     return;
   }
+
   if (!comp->HasICData() || (comp->ic_data()->NumberOfChecks() == 0)) {
     return;
   }
+
   ASSERT(comp->ic_data()->num_args_tested() == 2);
   if (comp->ic_data()->NumberOfChecks() == 1) {
     GrowableArray<intptr_t> class_ids;
@@ -2267,6 +2319,8 @@ void FlowGraphOptimizer::HandleEqualityCompare(EqualityCompareInstr* comp,
 }
 
 
+
+
 void FlowGraphOptimizer::VisitEqualityCompare(EqualityCompareInstr* instr) {
   HandleEqualityCompare(instr, instr);
 }