Faster checks in polymorphic instance calls if Smi-s are involved

runtime/vm/flow_graph_compiler_arm.cc

Index: runtime/vm/flow_graph_compiler_arm.cc
diff --git a/runtime/vm/flow_graph_compiler_arm.cc b/runtime/vm/flow_graph_compiler_arm.cc
index 8c6c4113aeb9d786489c44d46ca0c3fac3a6fc2c..7e9f26d486ebc5be27b8d736fbb077ed7a4b2019 100644
--- a/runtime/vm/flow_graph_compiler_arm.cc
+++ b/runtime/vm/flow_graph_compiler_arm.cc
@@ -1512,33 +1512,72 @@ void FlowGraphCompiler::ClobberDeadTempRegisters(LocationSummary* locs) {
 
 
 void FlowGraphCompiler::EmitTestAndCall(const ICData& ic_data,
-                                        Register class_id_reg,
                                         intptr_t argument_count,
                                         const Array& argument_names,
-                                        Label* deopt,
+                                        Label* failed,
+                                        Label* match_found,
                                         intptr_t deopt_id,
                                         intptr_t token_index,
                                         LocationSummary* locs) {
   ASSERT(is_optimizing());
-  ASSERT(!ic_data.IsNull() && (ic_data.NumberOfUsedChecks() > 0));
-  Label match_found;
-  const intptr_t len = ic_data.NumberOfChecks();
-  GrowableArray<CidTarget> sorted(len);
-  SortICDataByCount(ic_data, &sorted, /* drop_smi = */ false);
-  ASSERT(class_id_reg != R4);
-  ASSERT(len > 0);  // Why bother otherwise.
+  __ Comment("EmitTestAndCall");
   const Array& arguments_descriptor =
       Array::ZoneHandle(ArgumentsDescriptor::New(argument_count,
                                                  argument_names));
   StubCode* stub_code = isolate()->stub_code();
 
+  // Load receiver into R0.
+  __ LoadFromOffset(kWord, R0, SP, (argument_count - 1) * kWordSize);
   __ LoadObject(R4, arguments_descriptor);
-  for (intptr_t i = 0; i < len; i++) {
-    const bool is_last_check = (i == (len - 1));
+
+  const bool kFirstCheckIsSmi = ic_data.GetReceiverClassIdAt(0) == kSmiCid;
+  const intptr_t kNumChecks = ic_data.NumberOfChecks();
+
+  ASSERT(!ic_data.IsNull() && (kNumChecks > 0));
+
+  Label after_smi_test;
+  __ tst(R0, Operand(kSmiTagMask));
+  if (kFirstCheckIsSmi) {
+    // Jump if receiver is not Smi.
+    if (kNumChecks == 1) {
+      __ b(failed, NE);
+    } else {
+      __ b(&after_smi_test, NE);
+    }
+    // Do not use the code from the function, but let the code be patched so
+    // that we can record the outgoing edges to other code.
+    GenerateDartCall(deopt_id,
+                     token_index,
+                     &stub_code->CallStaticFunctionLabel(),
+                     RawPcDescriptors::kOther,
+                     locs);
+    const Function& function = Function::Handle(ic_data.GetTargetAt(0));
+    AddStaticCallTarget(function);
+    __ Drop(argument_count);
+    if (kNumChecks > 1) {
+      __ b(match_found);
+    }
+  } else {
+    // It is Smi, butSmi is not handled here.

[zra, 2015/06/15 20:17:22]

butSmi -> but Smi

[srdjan, 2015/06/15 20:54:13]

Done.

+    __ b(failed, EQ);

[zra, 2015/06/15 20:17:22]

Why isn't this:

if (kNumChecks == 1) {
  __ b(failed, EQ);
} else { ... }

as above?

[srdjan, 2015/06/15 20:54:13]

Added/changed comment:

    // Receiver is Smi, but Smi is not a valid class therefore fail.
    // (Smi class must be first in the list).

+  }
+  __ Bind(&after_smi_test);
+
+  ASSERT(!ic_data.IsNull() && (kNumChecks > 0));
+  GrowableArray<CidTarget> sorted(kNumChecks);
+  SortICDataByCount(ic_data, &sorted, /* drop_smi = */ true);
+
+  // Value is not Smi,
+  const intptr_t kSortedLen = sorted.length();
+  if (kSortedLen == 0) return;

[zra, 2015/06/15 20:17:23]

Should this branch to the 'failed' label?

If the branch to failed on 1562 doesn't need to be rewritten as in my comment
there, then maybe this should be an ASSERT instead.

[srdjan, 2015/06/15 20:54:13]

Added comment:

  // If kSortedLen is 0 then only a Smi check was needed; the Smi check above
  // will fail if there was only one check and receiver is not Smi.

+
+  __ LoadClassId(R2, R0);
+  for (intptr_t i = 0; i < kSortedLen; i++) {
+    const bool kIsLastCheck = (i == (kSortedLen - 1));
     Label next_test;

[srdjan, 2015/06/15 20:54:13]

Added assert that sorted[i].cid is not kSMiCid

-    __ CompareImmediate(class_id_reg, sorted[i].cid);
-    if (is_last_check) {
-      __ b(deopt, NE);
+    __ CompareImmediate(R2, sorted[i].cid);
+    if (kIsLastCheck) {
+      __ b(failed, NE);
     } else {
       __ b(&next_test, NE);
     }
@@ -1552,12 +1591,11 @@ void FlowGraphCompiler::EmitTestAndCall(const ICData& ic_data,
     const Function& function = *sorted[i].target;
     AddStaticCallTarget(function);
     __ Drop(argument_count);
-    if (!is_last_check) {
-      __ b(&match_found);
+    if (!kIsLastCheck) {
+      __ b(match_found);
     }
     __ Bind(&next_test);
   }
-  __ Bind(&match_found);
 }