[--runtime-call-stats] Fix ACCESSOR handler computation

When running with FLAG_runtime_call_stats, native accessor accesses must
go through the runtime for accurate accounting. Previously the slow_stub()
was used as a handler in order to accomplish this, but it could never be
looked up from the code cache successfully due to mismatched code flags,
which could cause more handler recompilations than in normal operation.
This patch fixes that by emitting a runtime call into the compiled
handler instead of using the slow_stub().

Drive-by cleanup: drop the unused StoreIC_Megamorphic builtin.

Review-Url: https://codereview.chromium.org/2054133002
Cr-Commit-Position: refs/heads/master@{#36926}

src/builtins.cc

@@ -5418,14 +5418,6 @@ void Generate_StoreIC_Setter_ForDeopt(MacroAssembler* masm) {
   NamedStoreHandlerCompiler::GenerateStoreViaSetterForDeopt(masm);
 }
 
-void Generate_StoreIC_Megamorphic(MacroAssembler* masm) {
-  StoreIC::GenerateMegamorphic(masm);
-}
-
-void Generate_StoreIC_Megamorphic_Strict(MacroAssembler* masm) {
-  StoreIC::GenerateMegamorphic(masm);
-}
-
 void Generate_KeyedStoreIC_Megamorphic(MacroAssembler* masm) {
   KeyedStoreIC::GenerateMegamorphic(masm, SLOPPY);
 }

src/builtins.h

@@ -260,9 +260,6 @@ inline bool operator&(BuiltinExtraArguments lhs, BuiltinExtraArguments rhs) {
                                                                              \
   V(StoreIC_Setter_ForDeopt, STORE_IC, StoreICState::kStrictModeState)       \
                                                                              \
-  V(StoreIC_Megamorphic, STORE_IC, kNoExtraICState)                          \
-  V(StoreIC_Megamorphic_Strict, STORE_IC, StoreICState::kStrictModeState)    \
-                                                                             \
   V(KeyedStoreIC_Megamorphic, KEYED_STORE_IC, kNoExtraICState)               \
   V(KeyedStoreIC_Megamorphic_Strict, KEYED_STORE_IC,                         \
     StoreICState::kStrictModeState)                                          \

src/counters.cc

@@ -285,8 +285,17 @@ void RuntimeCallStats::Enter(Isolate* isolate, RuntimeCallTimer* timer,
 // static
 void RuntimeCallStats::Leave(Isolate* isolate, RuntimeCallTimer* timer) {
   RuntimeCallStats* stats = isolate->counters()->runtime_call_stats();
-  DCHECK_EQ(stats->current_timer_, timer);
+
+  if (stats->current_timer_ == timer) {
     stats->current_timer_ = timer->Stop();
+  } else {
+    // Must be a Threading cctest. Walk the chain of Timers to find the
+    // buried one that's leaving. We don't care about keeping nested timings
+    // accurate, just avoid crashing by keeping the chain intact.
+    RuntimeCallTimer* next = stats->current_timer_;
+    while (next->parent_ != timer) next = next->parent_;
+    next->parent_ = timer->Stop();
+  }
 }
 
 // static

src/ic/arm/ic-arm.cc

@@ -734,26 +734,6 @@ void KeyedStoreIC::GenerateMegamorphic(MacroAssembler* masm,
   GenerateMiss(masm);
 }
 
-
-void StoreIC::GenerateMegamorphic(MacroAssembler* masm) {
-  Register receiver = StoreDescriptor::ReceiverRegister();
-  Register name = StoreDescriptor::NameRegister();
-  DCHECK(receiver.is(r1));
-  DCHECK(name.is(r2));
-  DCHECK(StoreDescriptor::ValueRegister().is(r0));
-
-  // Get the receiver from the stack and probe the stub cache.
-  Code::Flags flags =
-      Code::RemoveHolderFromFlags(Code::ComputeHandlerFlags(Code::STORE_IC));
-
-  masm->isolate()->stub_cache()->GenerateProbe(masm, Code::STORE_IC, flags,
-                                               receiver, name, r5, r6, r7, r8);
-
-  // Cache miss: Jump to runtime.
-  GenerateMiss(masm);
-}
-
-
 void StoreIC::GenerateMiss(MacroAssembler* masm) {
   StoreIC_PushArgs(masm);
 

src/ic/arm64/ic-arm64.cc

@@ -724,24 +724,6 @@ void KeyedStoreIC::GenerateMegamorphic(MacroAssembler* masm,
   GenerateMiss(masm);
 }
 
-
-void StoreIC::GenerateMegamorphic(MacroAssembler* masm) {
-  Register receiver = StoreDescriptor::ReceiverRegister();
-  Register name = StoreDescriptor::NameRegister();
-  DCHECK(!AreAliased(receiver, name, StoreDescriptor::ValueRegister(), x5, x6,
-                     x7, x8));
-
-  // Probe the stub cache.
-  Code::Flags flags =
-      Code::RemoveHolderFromFlags(Code::ComputeHandlerFlags(Code::STORE_IC));
-  masm->isolate()->stub_cache()->GenerateProbe(masm, Code::STORE_IC, flags,
-                                               receiver, name, x5, x6, x7, x8);
-
-  // Cache miss: Jump to runtime.
-  GenerateMiss(masm);
-}
-
-
 void StoreIC::GenerateMiss(MacroAssembler* masm) {
   StoreIC_PushArgs(masm);
 

src/ic/handler-compiler.cc

@@ -226,7 +226,11 @@ Handle<Code> NamedLoadHandlerCompiler::CompileLoadNonexistent(
 Handle<Code> NamedLoadHandlerCompiler::CompileLoadCallback(
     Handle<Name> name, Handle<AccessorInfo> callback) {
   Register reg = Frontend(name);
+  if (FLAG_runtime_call_stats) {
+    TailCallBuiltin(masm(), Builtins::kLoadIC_Slow);
+  } else {
     GenerateLoadCallback(reg, callback);
+  }
   return GetCode(kind(), name);
 }
 
@@ -236,8 +240,12 @@ Handle<Code> NamedLoadHandlerCompiler::CompileLoadCallback(
     int accessor_index) {
   DCHECK(call_optimization.is_simple_api_call());
   Register holder = Frontend(name);
+  if (FLAG_runtime_call_stats) {
+    TailCallBuiltin(masm(), Builtins::kLoadIC_Slow);
+  } else {
     GenerateApiAccessorCall(masm(), call_optimization, map(), receiver(),
                             scratch2(), false, no_reg, holder, accessor_index);
+  }
   return GetCode(kind(), name);
 }
 
@@ -561,9 +569,14 @@ Handle<Code> NamedStoreHandlerCompiler::CompileStoreCallback(
     Handle<JSObject> object, Handle<Name> name,
     const CallOptimization& call_optimization, int accessor_index) {
   Register holder = Frontend(name);
+  if (FLAG_runtime_call_stats) {
+    GenerateRestoreName(name);
+    TailCallBuiltin(masm(), Builtins::kStoreIC_Slow);
+  } else {
     GenerateApiAccessorCall(masm(), call_optimization, handle(object->map()),
                             receiver(), scratch2(), true, value(), holder,
                             accessor_index);
+  }
   return GetCode(kind(), name);
 }
 

src/ic/ia32/ic-ia32.cc

@@ -708,15 +708,6 @@ void KeyedLoadIC::GenerateRuntimeGetProperty(MacroAssembler* masm) {
   __ TailCallRuntime(Runtime::kKeyedGetProperty);
 }
 
-
-void StoreIC::GenerateMegamorphic(MacroAssembler* masm) {
-  // This shouldn't be called.
-  // TODO(mvstanton): remove this method.
-  __ int3();
-  return;
-}
-
-
 static void StoreIC_PushArgs(MacroAssembler* masm) {
   Register receiver = StoreDescriptor::ReceiverRegister();
   Register name = StoreDescriptor::NameRegister();

src/ic/ic.cc

@@ -1198,7 +1198,6 @@ Handle<Code> LoadIC::CompileHandler(LookupIterator* lookup,
           int index = lookup->GetAccessorIndex();
           Handle<Code> code = compiler.CompileLoadCallback(
               lookup->name(), call_optimization, index);
-          if (FLAG_runtime_call_stats) return slow_stub();
           return code;
         }
         TRACE_HANDLER_STATS(isolate(), LoadIC_LoadViaGetter);
@@ -1218,7 +1217,6 @@ Handle<Code> LoadIC::CompileHandler(LookupIterator* lookup,
         TRACE_HANDLER_STATS(isolate(), LoadIC_LoadCallback);
         NamedLoadHandlerCompiler compiler(isolate(), map, holder, cache_holder);
         Handle<Code> code = compiler.CompileLoadCallback(lookup->name(), info);
-        if (FLAG_runtime_call_stats) return slow_stub();
         return code;
       }
       UNREACHABLE();
@@ -1565,11 +1563,6 @@ Handle<Code> StoreIC::initialize_stub_in_optimized_code(
   return stub.GetCode();
 }
 
-Handle<Code> StoreIC::slow_stub() const {
-  return isolate()->builtins()->StoreIC_Slow();
-}
-
-
 void StoreIC::UpdateCaches(LookupIterator* lookup, Handle<Object> value,
                            JSReceiver::StoreFromKeyed store_mode) {
   if (state() == UNINITIALIZED) {
@@ -1788,7 +1781,6 @@ Handle<Code> StoreIC::CompileHandler(LookupIterator* lookup,
         NamedStoreHandlerCompiler compiler(isolate(), receiver_map(), holder);
         Handle<Code> code = compiler.CompileStoreCallback(
             receiver, lookup->name(), info, language_mode());
-        if (FLAG_runtime_call_stats) return slow_stub();
         return code;
       } else {
         DCHECK(accessors->IsAccessorPair());
@@ -1803,7 +1795,6 @@ Handle<Code> StoreIC::CompileHandler(LookupIterator* lookup,
           Handle<Code> code = compiler.CompileStoreCallback(
               receiver, lookup->name(), call_optimization,
               lookup->GetAccessorIndex());
-          if (FLAG_runtime_call_stats) return slow_stub();
           return code;
         }
         TRACE_HANDLER_STATS(isolate(), StoreIC_StoreViaSetter);
@@ -2743,6 +2734,12 @@ RUNTIME_FUNCTION(Runtime_StoreCallbackProperty) {
   CONVERT_LANGUAGE_MODE_ARG_CHECKED(language_mode, 5);
   HandleScope scope(isolate);
 
+  if (FLAG_runtime_call_stats) {
+    RETURN_RESULT_OR_FAILURE(
+        isolate, Runtime::SetObjectProperty(isolate, receiver, name, value,
+                                            language_mode));
+  }
+
   Handle<AccessorInfo> callback(
       callback_or_cell->IsWeakCell()
           ? AccessorInfo::cast(WeakCell::cast(*callback_or_cell)->value())

src/ic/ic.h

@@ -363,10 +363,7 @@ class StoreIC : public IC {
   // Code generators for stub routines. Only called once at startup.
   static void GenerateSlow(MacroAssembler* masm);
   static void GenerateMiss(MacroAssembler* masm);
-  static void GenerateMegamorphic(MacroAssembler* masm);
   static void GenerateNormal(MacroAssembler* masm);
-  static void GenerateRuntimeSetProperty(MacroAssembler* masm,
-                                         LanguageMode language_mode);
 
   static Handle<Code> initialize_stub_in_optimized_code(
       Isolate* isolate, LanguageMode language_mode);
@@ -383,7 +380,9 @@ class StoreIC : public IC {
 
  protected:
   // Stub accessors.
-  Handle<Code> slow_stub() const;
+  Handle<Code> slow_stub() const {
+    return isolate()->builtins()->StoreIC_Slow();
+  }
 
   // Update the inline cache and the global stub cache based on the
   // lookup result.

src/ic/mips/ic-mips.cc

@@ -727,25 +727,6 @@ void KeyedStoreIC::GenerateMiss(MacroAssembler* masm) {
   __ TailCallRuntime(Runtime::kKeyedStoreIC_Miss);
 }
 
-
-void StoreIC::GenerateMegamorphic(MacroAssembler* masm) {
-  Register receiver = StoreDescriptor::ReceiverRegister();
-  Register name = StoreDescriptor::NameRegister();
-  DCHECK(receiver.is(a1));
-  DCHECK(name.is(a2));
-  DCHECK(StoreDescriptor::ValueRegister().is(a0));
-
-  // Get the receiver from the stack and probe the stub cache.
-  Code::Flags flags =
-      Code::RemoveHolderFromFlags(Code::ComputeHandlerFlags(Code::STORE_IC));
-  masm->isolate()->stub_cache()->GenerateProbe(masm, Code::STORE_IC, flags,
-                                               receiver, name, t1, t2, t3, t4);
-
-  // Cache miss: Jump to runtime.
-  GenerateMiss(masm);
-}
-
-
 void StoreIC::GenerateMiss(MacroAssembler* masm) {
   StoreIC_PushArgs(masm);
 

src/ic/mips64/ic-mips64.cc

@@ -731,25 +731,6 @@ void KeyedStoreIC::GenerateMiss(MacroAssembler* masm) {
   __ TailCallRuntime(Runtime::kKeyedStoreIC_Miss);
 }
 
-
-void StoreIC::GenerateMegamorphic(MacroAssembler* masm) {
-  Register receiver = StoreDescriptor::ReceiverRegister();
-  Register name = StoreDescriptor::NameRegister();
-  DCHECK(receiver.is(a1));
-  DCHECK(name.is(a2));
-  DCHECK(StoreDescriptor::ValueRegister().is(a0));
-
-  // Get the receiver from the stack and probe the stub cache.
-  Code::Flags flags =
-      Code::RemoveHolderFromFlags(Code::ComputeHandlerFlags(Code::STORE_IC));
-  masm->isolate()->stub_cache()->GenerateProbe(masm, Code::STORE_IC, flags,
-                                               receiver, name, a5, a6, a7, t0);
-
-  // Cache miss: Jump to runtime.
-  GenerateMiss(masm);
-}
-
-
 void StoreIC::GenerateMiss(MacroAssembler* masm) {
   StoreIC_PushArgs(masm);
 

src/ic/ppc/ic-ppc.cc

@@ -737,26 +737,6 @@ void KeyedStoreIC::GenerateMegamorphic(MacroAssembler* masm,
   GenerateMiss(masm);
 }
 
-
-void StoreIC::GenerateMegamorphic(MacroAssembler* masm) {
-  Register receiver = StoreDescriptor::ReceiverRegister();
-  Register name = StoreDescriptor::NameRegister();
-  DCHECK(receiver.is(r4));
-  DCHECK(name.is(r5));
-  DCHECK(StoreDescriptor::ValueRegister().is(r3));
-
-  // Get the receiver from the stack and probe the stub cache.
-  Code::Flags flags =
-      Code::RemoveHolderFromFlags(Code::ComputeHandlerFlags(Code::STORE_IC));
-
-  masm->isolate()->stub_cache()->GenerateProbe(
-      masm, Code::STORE_IC, flags, receiver, name, r8, r9, r10, r11);
-
-  // Cache miss: Jump to runtime.
-  GenerateMiss(masm);
-}
-
-
 void StoreIC::GenerateMiss(MacroAssembler* masm) {
   StoreIC_PushArgs(masm);
 

src/ic/s390/ic-s390.cc

@@ -719,24 +719,6 @@ void KeyedStoreIC::GenerateMegamorphic(MacroAssembler* masm,
   GenerateMiss(masm);
 }
 
-void StoreIC::GenerateMegamorphic(MacroAssembler* masm) {
-  Register receiver = StoreDescriptor::ReceiverRegister();
-  Register name = StoreDescriptor::NameRegister();
-  DCHECK(receiver.is(r3));
-  DCHECK(name.is(r4));
-  DCHECK(StoreDescriptor::ValueRegister().is(r2));
-
-  // Get the receiver from the stack and probe the stub cache.
-  Code::Flags flags =
-      Code::RemoveHolderFromFlags(Code::ComputeHandlerFlags(Code::STORE_IC));
-
-  masm->isolate()->stub_cache()->GenerateProbe(masm, Code::STORE_IC, flags,
-                                               receiver, name, r7, r8, r9, ip);
-
-  // Cache miss: Jump to runtime.
-  GenerateMiss(masm);
-}
-
 void StoreIC::GenerateMiss(MacroAssembler* masm) {
   StoreIC_PushArgs(masm);
 

src/ic/x64/ic-x64.cc

@@ -710,13 +710,6 @@ void KeyedLoadIC::GenerateRuntimeGetProperty(MacroAssembler* masm) {
   __ TailCallRuntime(Runtime::kKeyedGetProperty);
 }
 
-
-void StoreIC::GenerateMegamorphic(MacroAssembler* masm) {
-  // This shouldn't be called.
-  __ int3();
-}
-
-
 static void StoreIC_PushArgs(MacroAssembler* masm) {
   Register receiver = StoreDescriptor::ReceiverRegister();
   Register name = StoreDescriptor::NameRegister();

src/ic/x87/ic-x87.cc

@@ -708,15 +708,6 @@ void KeyedLoadIC::GenerateRuntimeGetProperty(MacroAssembler* masm) {
   __ TailCallRuntime(Runtime::kKeyedGetProperty);
 }
 
-
-void StoreIC::GenerateMegamorphic(MacroAssembler* masm) {
-  // This shouldn't be called.
-  // TODO(mvstanton): remove this method.
-  __ int3();
-  return;
-}
-
-
 static void StoreIC_PushArgs(MacroAssembler* masm) {
   Register receiver = StoreDescriptor::ReceiverRegister();
   Register name = StoreDescriptor::NameRegister();