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

runtime/vm/flow_graph_compiler_arm.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/globals.h"  // Needed here to get TARGET_ARCH_ARM.
 #if defined(TARGET_ARCH_ARM)
 
 #include "vm/flow_graph_compiler.h"
 
 #include "vm/ast_printer.h"
@@ skipping 1494 matching lines @@
     if (tmp.IsRegister() &&
         !locs->live_registers()->ContainsRegister(tmp.reg())) {
       __ mov(tmp.reg(), Operand(0xf7));
     }
   }
 }
 #endif
 
 
 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);
     }
     // 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 = *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);
 }
 
 
 #undef __
 #define __ compiler_->assembler()->
 
 
 void ParallelMoveResolver::EmitMove(int index) {
   MoveOperands* move = moves_[index];
   const Location source = move->src();
@@ skipping 286 matching lines @@
   DRegister dreg = EvenDRegisterOf(reg);
   __ vldrd(dreg, Address(SP, kDoubleSize, Address::PostIndex));
 }
 
 
 #undef __
 
 }  // namespace dart
 
 #endif  // defined TARGET_ARCH_ARM