Allow equality operation on mixed double/smi arguments.

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 585 matching lines @@
 // Returns false if the ICData contains anything other than the 4 combinations
 // of Mint and Smi for the receiver and argument classes.
 static bool HasTwoMintOrSmi(const ICData& ic_data) {
   GrowableArray<intptr_t> class_ids(2);
   class_ids.Add(kSmiCid);
   class_ids.Add(kMintCid);
   return ICDataHasOnlyReceiverArgumentClassIds(ic_data, class_ids, class_ids);
 }
 
 
+// Returns false if the ICData contains anything other than the 4 combinations
+// of Double and Smi for the receiver and argument classes.
+static bool HasTwoDoubleOrSmi(const ICData& ic_data) {
+  GrowableArray<intptr_t> class_ids(2);
+  class_ids.Add(kSmiCid);
+  class_ids.Add(kDoubleCid);
+  return ICDataHasOnlyReceiverArgumentClassIds(ic_data, class_ids, class_ids);
+}
+
+
 static bool HasOnlyOneDouble(const ICData& ic_data) {
   return (ic_data.NumberOfChecks() == 1)
       && ic_data.HasReceiverClassId(kDoubleCid);
 }
 
 
 static bool ShouldSpecializeForDouble(const ICData& ic_data) {
   // Unboxed double operation can't handle case of two smis.
   if (ICDataHasReceiverArgumentClassIds(ic_data, kSmiCid, kSmiCid)) {
     return false;
@@ skipping 2022 matching lines @@
       new Value(instr->ArgumentAt(1)),
       needs_store_barrier);
   // Discard the environment from the original instruction because the store
   // can't deoptimize.
   instr->RemoveEnvironment();
   ReplaceCall(instr, store);
   return true;
 }
 
 
+static bool SmiFitsInDouble() { return kSmiBits < 53; }
+
+
 void FlowGraphOptimizer::HandleRelationalOp(RelationalOpInstr* comp) {
   if (!comp->HasICData() || (comp->ic_data()->NumberOfChecks() == 0)) {
     return;
   }
   const ICData& ic_data = *comp->ic_data();

[Cutch, 2013/07/25 21:03:36]

To match HandleEqualityCompare why don't you add:

ASSERT(ic_data.num_args_tested() == 2);
ASSERT(comp->operation_cid() == kIllegalCid);

[srdjan, 2013/07/25 22:17:02]

Factored out code. Done.

   Instruction* instr = current_iterator()->Current();
-  if (ic_data.NumberOfChecks() == 1) {
-    ASSERT(ic_data.HasOneTarget());
-    if (HasOnlyTwoSmis(ic_data)) {
-      InsertBefore(instr,
-                   new CheckSmiInstr(comp->left()->Copy(), comp->deopt_id()),
-                   instr->env(),
-                   Definition::kEffect);
-      InsertBefore(instr,
-                   new CheckSmiInstr(comp->right()->Copy(), comp->deopt_id()),
-                   instr->env(),
-                   Definition::kEffect);
-      comp->set_operands_class_id(kSmiCid);
-    } else if (ShouldSpecializeForDouble(ic_data)) {
-      comp->set_operands_class_id(kDoubleCid);
-    } else if (HasTwoMintOrSmi(*comp->ic_data()) &&
-               FlowGraphCompiler::SupportsUnboxedMints()) {
-      comp->set_operands_class_id(kMintCid);
+  if (HasOnlyTwoSmis(ic_data)) {
+    InsertBefore(instr,
+                 new CheckSmiInstr(comp->left()->Copy(), comp->deopt_id()),
+                 instr->env(),
+                 Definition::kEffect);
+    InsertBefore(instr,
+                 new CheckSmiInstr(comp->right()->Copy(), comp->deopt_id()),
+                 instr->env(),
+                 Definition::kEffect);
+    comp->set_operation_cid(kSmiCid);
+  } else if (HasTwoMintOrSmi(ic_data) &&
+             FlowGraphCompiler::SupportsUnboxedMints()) {
+    comp->set_operation_cid(kMintCid);
+  } else if (HasTwoDoubleOrSmi(ic_data)) {
+    // Use double comparison.
+    if (SmiFitsInDouble()) {
+      comp->set_operation_cid(kDoubleCid);
     } else {
-      ASSERT(comp->operands_class_id() == kIllegalCid);
+      if (ICDataHasReceiverArgumentClassIds(ic_data, kSmiCid, kSmiCid)) {
+        // We cannot use double comparison on two Smi-s.
+        ASSERT(comp->operation_cid() == kIllegalCid);
+      } else {
+        InsertBefore(instr,
+                     new CheckEitherNonSmiInstr(comp->left()->Copy(),
+                                                comp->right()->Copy(),
+                                                comp->deopt_id()),
+                     instr->env(),
+                     Definition::kEffect);
+        comp->set_operation_cid(kDoubleCid);
+      }
     }
-  } else if (HasTwoMintOrSmi(*comp->ic_data()) &&
-             FlowGraphCompiler::SupportsUnboxedMints()) {
-    comp->set_operands_class_id(kMintCid);
+  } else {
+    ASSERT(comp->operation_cid() == kIllegalCid);
   }
 }
 
 
 void FlowGraphOptimizer::VisitRelationalOp(RelationalOpInstr* instr) {
   HandleRelationalOp(instr);
 }
 
 
 bool FlowGraphOptimizer::CanStrictifyEqualityCompare(
@@ skipping 64 matching lines @@
 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;
-    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()),
-                   current_instruction->env(),
-                   Definition::kEffect);
-      InsertBefore(current_instruction,
-                   new CheckSmiInstr(comp->right()->Copy(), comp->deopt_id()),
-                   current_instruction->env(),
-                   Definition::kEffect);
-      comp->set_receiver_class_id(kSmiCid);
-    } else if ((class_ids[0] == kDoubleCid) && (class_ids[1] == kDoubleCid)) {
-      comp->set_receiver_class_id(kDoubleCid);
-    } else if (HasTwoMintOrSmi(*comp->ic_data()) &&
-               FlowGraphCompiler::SupportsUnboxedMints()) {
-      comp->set_receiver_class_id(kMintCid);
+  const ICData& ic_data = *comp->ic_data();
+  ASSERT(ic_data.num_args_tested() == 2);

[Cutch, 2013/07/25 21:03:36]

Can this duplicate code in HandleEqualityCompare and HandleRelationalOp be moved
into a common function?

[srdjan, 2013/07/25 22:17:02]

Done.

+  ASSERT(comp->operation_cid() == kIllegalCid);
+  if (HasOnlyTwoSmis(ic_data)) {
+    InsertBefore(current_instruction,
+                 new CheckSmiInstr(comp->left()->Copy(), comp->deopt_id()),
+                 current_instruction->env(),
+                 Definition::kEffect);
+    InsertBefore(current_instruction,
+                 new CheckSmiInstr(comp->right()->Copy(), comp->deopt_id()),
+                 current_instruction->env(),
+                 Definition::kEffect);
+    comp->set_operation_cid(kSmiCid);
+  } else if (HasTwoMintOrSmi(ic_data) &&
+             FlowGraphCompiler::SupportsUnboxedMints()) {
+    comp->set_operation_cid(kMintCid);
+  } else if (HasTwoDoubleOrSmi(ic_data)) {
+    // Use double comparison.
+    if (SmiFitsInDouble()) {
+      comp->set_operation_cid(kDoubleCid);
     } else {
-      ASSERT(comp->receiver_class_id() == kIllegalCid);
+      if (ICDataHasReceiverArgumentClassIds(ic_data, kSmiCid, kSmiCid)) {
+        // We cannot use double comparison on two Smi-s.
+        ASSERT(comp->operation_cid() == kIllegalCid);
+      } else {
+        InsertBefore(current_instruction,
+                     new CheckEitherNonSmiInstr(comp->left()->Copy(),
+                                                comp->right()->Copy(),
+                                                comp->deopt_id()),
+                     current_instruction->env(),
+                     Definition::kEffect);
+        comp->set_operation_cid(kDoubleCid);
+      }
     }
-  } else if (HasTwoMintOrSmi(*comp->ic_data()) &&
-             FlowGraphCompiler::SupportsUnboxedMints()) {
-    comp->set_receiver_class_id(kMintCid);
+  } else {
+    ASSERT(comp->operation_cid() == kIllegalCid);
   }
 
-  if (comp->receiver_class_id() != kIllegalCid) {
+  if (comp->operation_cid() != kIllegalCid) {
     // Done.
     return;
   }
 
   // Check if ICDData contains checks with Smi/Null combinations. In that case
   // we can still emit the optimized Smi equality operation but need to add
   // checks for null or Smi.
   // TODO(srdjan): Add it for Double and Mint.
   GrowableArray<intptr_t> smi_or_null(2);
   smi_or_null.Add(kSmiCid);
   smi_or_null.Add(kNullCid);
-  if (ICDataHasOnlyReceiverArgumentClassIds(*comp->ic_data(),
+  if (ICDataHasOnlyReceiverArgumentClassIds(ic_data,
                                             smi_or_null,
                                             smi_or_null)) {
     const ICData& unary_checks_0 =
         ICData::ZoneHandle(comp->ic_data()->AsUnaryClassChecks());
     AddCheckClass(comp->left()->definition(),
                   unary_checks_0,
                   comp->deopt_id(),
                   current_instruction->env(),
                   current_instruction);
 
     const ICData& unary_checks_1 =
         ICData::ZoneHandle(comp->ic_data()->AsUnaryClassChecksForArgNr(1));
     AddCheckClass(comp->right()->definition(),
                   unary_checks_1,
                   comp->deopt_id(),
                   current_instruction->env(),
                   current_instruction);
-    comp->set_receiver_class_id(kSmiCid);
+    comp->set_operation_cid(kSmiCid);
   }
 }
 
 
 
 
 void FlowGraphOptimizer::VisitEqualityCompare(EqualityCompareInstr* instr) {
   HandleEqualityCompare(instr, instr);
 }
 
@@ skipping 299 matching lines @@
   flow_graph_->InsertAfter(after, constraint, NULL, Definition::kValue);
   RenameDominatedUses(defn, constraint, constraint);
   constraints_.Add(constraint);
   return constraint;
 }
 
 
 void RangeAnalysis::ConstrainValueAfterBranch(Definition* defn, Value* use) {
   BranchInstr* branch = use->instruction()->AsBranch();
   RelationalOpInstr* rel_op = branch->comparison()->AsRelationalOp();
-  if ((rel_op != NULL) && (rel_op->operands_class_id() == kSmiCid)) {
+  if ((rel_op != NULL) && (rel_op->operation_cid() == kSmiCid)) {
     // Found comparison of two smis. Constrain defn at true and false
     // successors using the other operand as a boundary.
     Definition* boundary;
     Token::Kind op_kind;
     if (use->use_index() == 0) {  // Left operand.
       boundary = rel_op->InputAt(1)->definition();
       op_kind = rel_op->kind();
     } else {
       ASSERT(use->use_index() == 1);  // Right operand.
       boundary = rel_op->InputAt(0)->definition();
@@ skipping 2622 matching lines @@
 
 
 void ConstantPropagator::VisitEqualityCompare(EqualityCompareInstr* instr) {
   const Object& left = instr->left()->definition()->constant_value();
   const Object& right = instr->right()->definition()->constant_value();
 
   if (instr->left()->definition() == instr->right()->definition()) {
     // Fold x == x, and x != x to true/false for numbers and checked strict
     // comparisons.
     if (instr->IsCheckedStrictEqual() ||
-        RawObject::IsIntegerClassId(instr->receiver_class_id())) {
+        RawObject::IsIntegerClassId(instr->operation_cid())) {
       return SetValue(instr,
                       (instr->kind() == Token::kEQ)
                         ? Bool::True()
                         : Bool::False());
     }
   }
 
   if (IsNonConstant(left) || IsNonConstant(right)) {
     SetValue(instr, non_constant_);
   } else if (IsConstant(left) && IsConstant(right)) {
@@ skipping 1449 matching lines @@
 
   // Insert materializations at environment uses.
   const Class& cls = Class::Handle(alloc->constructor().Owner());
   for (intptr_t i = 0; i < exits.length(); i++) {
     CreateMaterializationAt(exits[i], alloc, cls, *fields);
   }
 }
 
 
 }  // namespace dart