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

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/flow_graph_builder.h"
 #include "vm/flow_graph_compiler.h"
@@ skipping 52 matching lines @@
           }
         }
       } else if (instr->IsPolymorphicInstanceCall()) {
         SpecializePolymorphicInstanceCall(instr->AsPolymorphicInstanceCall());
       } else if (instr->IsStrictCompare()) {
         VisitStrictCompare(instr->AsStrictCompare());
       } else if (instr->IsBranch()) {
         ComparisonInstr* compare = instr->AsBranch()->comparison();
         if (compare->IsStrictCompare()) {
           VisitStrictCompare(compare->AsStrictCompare());
+        } else if (compare->IsEqualityCompare()) {
+          StrictifyEqualityCompare(compare->AsEqualityCompare(),
+                                   instr->AsBranch());
         }
       }
     }
     current_iterator_ = NULL;
   }
 }
 
 
 // Attempt to build ICData for call using propagated class-ids.
 bool FlowGraphOptimizer::TryCreateICData(InstanceCallInstr* call) {
@@ skipping 2094 matching lines @@
     comp->set_operands_class_id(kMintCid);
   }
 }
 
 
 void FlowGraphOptimizer::VisitRelationalOp(RelationalOpInstr* instr) {
   HandleRelationalOp(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>
+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,
+                               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 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) ?
-        Token::kEQ_STRICT : Token::kNE_STRICT;
-    StrictCompareInstr* strict_comp =
-        new StrictCompareInstr(strict_kind,
-                               comp->left()->Copy(),
-                               comp->right()->Copy());
-    current_instruction->ReplaceWith(strict_comp, current_iterator());
+  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;
     Function& target = Function::Handle();
     comp->ic_data()->GetCheckAt(0, &class_ids, &target);
     // TODO(srdjan): allow for mixed mode int/double comparison.
 
     if ((class_ids[0] == kSmiCid) && (class_ids[1] == kSmiCid)) {
       InsertBefore(current_instruction,
                    new CheckSmiInstr(comp->left()->Copy(), comp->deopt_id()),
@@ skipping 45 matching lines @@
     AddCheckClass(comp->right()->definition(),
                   unary_checks_1,
                   comp->deopt_id(),
                   current_instruction->env(),
                   current_instruction);
     comp->set_receiver_class_id(kSmiCid);
   }
 }
 
 
+
+
 void FlowGraphOptimizer::VisitEqualityCompare(EqualityCompareInstr* instr) {
   HandleEqualityCompare(instr, instr);
 }
 
 
 void FlowGraphOptimizer::VisitBranch(BranchInstr* instr) {
   ComparisonInstr* comparison = instr->comparison();
   if (comparison->IsRelationalOp()) {
     HandleRelationalOp(comparison->AsRelationalOp());
   } else if (comparison->IsEqualityCompare()) {
@@ skipping 2780 matching lines @@
   if (changed) {
     // We may have changed the block order and the dominator tree.
     flow_graph->DiscoverBlocks();
     GrowableArray<BitVector*> dominance_frontier;
     flow_graph->ComputeDominators(&dominance_frontier);
   }
 }
 
 
 }  // namespace dart