Move megamorphic lookup code to stub instead of inlining. Preformance loss < 5%, instruction size r…

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 10 matching lines @@
 #include "vm/symbols.h"
 #include "vm/verified_memory.h"
 
 namespace dart {
 
 DEFINE_FLAG(bool, trap_on_deoptimization, false, "Trap on deoptimization.");
 DEFINE_FLAG(bool, unbox_mints, true, "Optimize 64-bit integer arithmetic.");
 DEFINE_FLAG(bool, unbox_doubles, true, "Optimize double arithmetic.");
 DECLARE_FLAG(bool, enable_type_checks);
 DECLARE_FLAG(bool, enable_simd_inline);
+DECLARE_FLAG(bool, use_megamorphic_stub);
 
 
 FlowGraphCompiler::~FlowGraphCompiler() {
   // BlockInfos are zone-allocated, so their destructors are not called.
   // Verify the labels explicitly here.
   for (int i = 0; i < block_info_.length(); ++i) {
     ASSERT(!block_info_[i]->jump_label()->IsLinked());
   }
 }
 
@@ skipping 1241 matching lines @@
     intptr_t token_pos,
     LocationSummary* locs) {
   MegamorphicCacheTable* table = Isolate::Current()->megamorphic_cache_table();
   const String& name = String::Handle(ic_data.target_name());
   const Array& arguments_descriptor =
       Array::ZoneHandle(ic_data.arguments_descriptor());
   ASSERT(!arguments_descriptor.IsNull() && (arguments_descriptor.Length() > 0));
   const MegamorphicCache& cache =
       MegamorphicCache::ZoneHandle(table->Lookup(name, arguments_descriptor));
   __ LoadFromOffset(kWord, R0, SP, (argument_count - 1) * kWordSize);
-  __ LoadTaggedClassIdMayBeSmi(R0, R0);
+  __ LoadObject(R1, cache);
 
-  // R0: class ID of the receiver (smi).
-  __ LoadObject(R1, cache);
-  __ ldr(R2, FieldAddress(R1, MegamorphicCache::buckets_offset()));
-  __ ldr(R1, FieldAddress(R1, MegamorphicCache::mask_offset()));
-  // R2: cache buckets array.
-  // R1: mask.
-  __ mov(R3, Operand(R0));
+  if (FLAG_use_megamorphic_stub) {

[Florian Schneider, 2015/05/04 22:06:20]

To avoid performance regression in optimized code, maybe try always doing the
inline lookup in optimized code?

if (FLAG_use_megamorphic_stub && !is_optimizing()) {
  ...
} else {
  ...
}

[srdjan, 2015/05/05 07:15:29]

There are no measureable (< 0.5% diffs) in optimized code. Only when running
with --noopt can I measure  regression.  I think the code size saving is more
significant than such a small performance loss What do you think?

[srdjan, 2015/05/05 12:13:28]

Measured dart2js optimized: with out-of-line megamorphic lookup: ~4% reduction
in code size, < 1% loss in performance. Will not to add '!is_optimizing', let me
know if you disagree.

[Florian Schneider, 2015/05/05 16:34:43]

If the difference is not noticable in optimized code, I'm totally fine with
always using the stub.

+    StubCode* stub_code = isolate()->stub_code();
+    __ BranchLink(&stub_code->MegamorphicLookupLabel());
+  } else  {
+    // R0: class ID of the receiver (smi).
+    __ LoadTaggedClassIdMayBeSmi(R0, R0);
+    __ ldr(R2, FieldAddress(R1, MegamorphicCache::buckets_offset()));
+    __ ldr(R1, FieldAddress(R1, MegamorphicCache::mask_offset()));
+    // R2: cache buckets array.
+    // R1: mask.
+    __ mov(R3, Operand(R0));
 
-  Label loop, update, call_target_function;
-  __ b(&loop);
+    Label loop, update, call_target_function;
+    __ b(&loop);
 
-  __ Bind(&update);
-  __ add(R3, R3, Operand(Smi::RawValue(1)));
-  __ Bind(&loop);
-  __ and_(R3, R3, Operand(R1));
-  const intptr_t base = Array::data_offset();
-  // R3 is smi tagged, but table entries are two words, so LSL 2.
-  __ add(IP, R2, Operand(R3, LSL, 2));
-  __ ldr(R4, FieldAddress(IP, base));
+    __ Bind(&update);
+    __ add(R3, R3, Operand(Smi::RawValue(1)));
+    __ Bind(&loop);
+    __ and_(R3, R3, Operand(R1));
+    const intptr_t base = Array::data_offset();
+    // R3 is smi tagged, but table entries are two words, so LSL 2.
+    __ add(IP, R2, Operand(R3, LSL, 2));
+    __ ldr(R4, FieldAddress(IP, base));
 
-  ASSERT(kIllegalCid == 0);
-  __ tst(R4, Operand(R4));
-  __ b(&call_target_function, EQ);
-  __ cmp(R4, Operand(R0));
-  __ b(&update, NE);
+    ASSERT(kIllegalCid == 0);
+    __ tst(R4, Operand(R4));
+    __ b(&call_target_function, EQ);
+    __ cmp(R4, Operand(R0));
+    __ b(&update, NE);
 
-  __ Bind(&call_target_function);
-  // Call the target found in the cache.  For a class id match, this is a
-  // proper target for the given name and arguments descriptor.  If the
-  // illegal class id was found, the target is a cache miss handler that can
-  // be invoked as a normal Dart function.
-  __ add(IP, R2, Operand(R3, LSL, 2));
-  __ ldr(R0, FieldAddress(IP, base + kWordSize));
-  __ ldr(R1, FieldAddress(R0, Function::instructions_offset()));
+    __ Bind(&call_target_function);
+    // Call the target found in the cache.  For a class id match, this is a
+    // proper target for the given name and arguments descriptor.  If the
+    // illegal class id was found, the target is a cache miss handler that can
+    // be invoked as a normal Dart function.
+    __ add(IP, R2, Operand(R3, LSL, 2));
+    __ ldr(R0, FieldAddress(IP, base + kWordSize));
+    __ ldr(R1, FieldAddress(R0, Function::instructions_offset()));
+    __ AddImmediate(R1, Instructions::HeaderSize() - kHeapObjectTag);
+  }

[Florian Schneider, 2015/05/04 22:06:20]

Maybe you can factor this lookup code into a common code generation helper that
the stub can reuse? It would avoid the duplication between here and
stub_code_*.cc.

[srdjan, 2015/05/05 07:15:29]

Will do.

   __ LoadObject(R5, ic_data);
   __ LoadObject(R4, arguments_descriptor);
-  __ AddImmediate(R1, Instructions::HeaderSize() - kHeapObjectTag);
   __ blx(R1);
   AddCurrentDescriptor(RawPcDescriptors::kOther,
       Isolate::kNoDeoptId, token_pos);
   RecordSafepoint(locs);
   const intptr_t deopt_id_after = Isolate::ToDeoptAfter(deopt_id);
   if (is_optimizing()) {
     AddDeoptIndexAtCall(deopt_id_after, token_pos);
   } else {
     // Add deoptimization continuation point after the call and before the
     // arguments are removed.
@@ skipping 542 matching lines @@
   DRegister dreg = EvenDRegisterOf(reg);
   __ vldrd(dreg, Address(SP, kDoubleSize, Address::PostIndex));
 }
 
 
 #undef __
 
 }  // namespace dart
 
 #endif  // defined TARGET_ARCH_ARM