Allow equality operation on mixed double/smi arguments.

runtime/vm/flow_graph_optimizer.cc

Index: runtime/vm/flow_graph_optimizer.cc
===================================================================
--- runtime/vm/flow_graph_optimizer.cc	(revision 25478)
+++ runtime/vm/flow_graph_optimizer.cc	(working copy)
@@ -603,6 +603,16 @@
 }
 
 
+// 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);
@@ -2645,35 +2655,48 @@
 }
 
 
+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);
   }
 }
 
@@ -2758,37 +2781,45 @@
     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;
   }
@@ -2800,7 +2831,7 @@
   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 =
@@ -2818,7 +2849,7 @@
                   comp->deopt_id(),
                   current_instruction->env(),
                   current_instruction);
-    comp->set_receiver_class_id(kSmiCid);
+    comp->set_operation_cid(kSmiCid);
   }
 }
 
@@ -3138,7 +3169,7 @@
 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;
@@ -5781,7 +5812,7 @@
     // 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()