From a2687daa67d4fca2812b9af08a0f287cdb1fe869 Mon Sep 17 00:00:00 2001 From: Toon Verwaest Date: Thu, 23 Mar 2017 15:42:25 +0100 Subject: [PATCH] Reland "[ic] General cleanup after moving more ICs to data handlers" Removed the invalid DCHECKs altogether. BUG=v8:5561 Change-Id: I678b80a2f216a84333e3fe65843ca9dfc0bdf0d5 Reviewed-on: https://chromium-review.googlesource.com/458280 Commit-Queue: Toon Verwaest Reviewed-by: Igor Sheludko Cr-Commit-Position: refs/heads/master@{#44072} --- src/counters.h | 23 ----- src/ic/ic.cc | 253 ++++++++++++++----------------------------------- src/ic/ic.h | 14 +-- 3 files changed, 76 insertions(+), 214 deletions(-) diff --git a/src/counters.h b/src/counters.h index fc4cb288aa..39d97dd018 100644 --- a/src/counters.h +++ b/src/counters.h @@ -762,7 +762,6 @@ class RuntimeCallTimer final { V(UnexpectedStubMiss) #define FOR_EACH_HANDLER_COUNTER(V) \ - V(IC_HandlerCacheHit) \ V(KeyedLoadIC_LoadIndexedStringStub) \ V(KeyedLoadIC_LoadIndexedInterceptorStub) \ V(KeyedLoadIC_KeyedLoadSloppyArgumentsStub) \ @@ -774,32 +773,20 @@ class RuntimeCallTimer final { V(KeyedStoreIC_StoreFastElementStub) \ V(KeyedStoreIC_StoreElementStub) \ V(LoadIC_FunctionPrototypeStub) \ - V(LoadIC_HandlerCacheHit_AccessCheck) \ - V(LoadIC_HandlerCacheHit_Exotic) \ - V(LoadIC_HandlerCacheHit_Interceptor) \ - V(LoadIC_HandlerCacheHit_JSProxy) \ - V(LoadIC_HandlerCacheHit_NonExistent) \ V(LoadIC_HandlerCacheHit_Accessor) \ - V(LoadIC_HandlerCacheHit_Data) \ - V(LoadIC_HandlerCacheHit_Transition) \ V(LoadIC_LoadApiGetterDH) \ V(LoadIC_LoadApiGetterFromPrototypeDH) \ - V(LoadIC_LoadApiGetterStub) \ V(LoadIC_LoadCallback) \ V(LoadIC_LoadConstantDH) \ V(LoadIC_LoadConstantFromPrototypeDH) \ - V(LoadIC_LoadConstant) \ - V(LoadIC_LoadConstantStub) \ V(LoadIC_LoadFieldDH) \ V(LoadIC_LoadFieldFromPrototypeDH) \ - V(LoadIC_LoadField) \ V(LoadIC_LoadGlobalFromPrototypeDH) \ V(LoadIC_LoadIntegerIndexedExoticDH) \ V(LoadIC_LoadInterceptorDH) \ V(LoadIC_LoadNonMaskingInterceptorDH) \ V(LoadIC_LoadInterceptorFromPrototypeDH) \ V(LoadIC_LoadNonexistentDH) \ - V(LoadIC_LoadNonexistent) \ V(LoadIC_LoadNormalDH) \ V(LoadIC_LoadNormalFromPrototypeDH) \ V(LoadIC_LoadScriptContextFieldStub) \ @@ -808,27 +795,17 @@ class RuntimeCallTimer final { V(LoadIC_Premonomorphic) \ V(LoadIC_SlowStub) \ V(LoadIC_StringLengthStub) \ - V(StoreIC_HandlerCacheHit_AccessCheck) \ - V(StoreIC_HandlerCacheHit_Exotic) \ - V(StoreIC_HandlerCacheHit_Interceptor) \ - V(StoreIC_HandlerCacheHit_JSProxy) \ - V(StoreIC_HandlerCacheHit_NonExistent) \ V(StoreIC_HandlerCacheHit_Accessor) \ - V(StoreIC_HandlerCacheHit_Data) \ - V(StoreIC_HandlerCacheHit_Transition) \ V(StoreIC_NonReceiver) \ V(StoreIC_Premonomorphic) \ V(StoreIC_SlowStub) \ V(StoreIC_StoreCallback) \ - V(StoreIC_StoreField) \ V(StoreIC_StoreFieldDH) \ - V(StoreIC_StoreFieldStub) \ V(StoreIC_StoreGlobalDH) \ V(StoreIC_StoreGlobalTransitionDH) \ V(StoreIC_StoreInterceptorStub) \ V(StoreIC_StoreNormalDH) \ V(StoreIC_StoreScriptContextFieldStub) \ - V(StoreIC_StoreTransition) \ V(StoreIC_StoreTransitionDH) \ V(StoreIC_StoreViaSetter) diff --git a/src/ic/ic.cc b/src/ic/ic.cc index 8b0733b98e..4daf1662d5 100644 --- a/src/ic/ic.cc +++ b/src/ic/ic.cc @@ -736,7 +736,7 @@ bool IC::IsTransitionOfMonomorphicTarget(Map* source_map, Map* target_map) { void IC::PatchCache(Handle name, Handle handler) { DCHECK(IsHandler(*handler)); // Currently only load and store ICs support non-code handlers. - DCHECK_IMPLIES(!handler->IsCode(), IsAnyLoad() || IsAnyStore()); + DCHECK(IsAnyLoad() || IsAnyStore()); switch (state()) { case UNINITIALIZED: case PREMONOMORPHIC: @@ -1017,69 +1017,17 @@ void IC::UpdateMegamorphicCache(Map* map, Name* name, Object* handler) { } void IC::TraceHandlerCacheHitStats(LookupIterator* lookup) { - if (!FLAG_runtime_call_stats) return; - + DCHECK_EQ(LookupIterator::ACCESSOR, lookup->state()); + if (V8_LIKELY(!FLAG_runtime_stats)) return; if (IsAnyLoad()) { - switch (lookup->state()) { - case LookupIterator::ACCESS_CHECK: - TRACE_HANDLER_STATS(isolate(), LoadIC_HandlerCacheHit_AccessCheck); - break; - case LookupIterator::INTEGER_INDEXED_EXOTIC: - TRACE_HANDLER_STATS(isolate(), LoadIC_HandlerCacheHit_Exotic); - break; - case LookupIterator::INTERCEPTOR: - TRACE_HANDLER_STATS(isolate(), LoadIC_HandlerCacheHit_Interceptor); - break; - case LookupIterator::JSPROXY: - TRACE_HANDLER_STATS(isolate(), LoadIC_HandlerCacheHit_JSProxy); - break; - case LookupIterator::NOT_FOUND: - TRACE_HANDLER_STATS(isolate(), LoadIC_HandlerCacheHit_NonExistent); - break; - case LookupIterator::ACCESSOR: - TRACE_HANDLER_STATS(isolate(), LoadIC_HandlerCacheHit_Accessor); - break; - case LookupIterator::DATA: - TRACE_HANDLER_STATS(isolate(), LoadIC_HandlerCacheHit_Data); - break; - case LookupIterator::TRANSITION: - TRACE_HANDLER_STATS(isolate(), LoadIC_HandlerCacheHit_Transition); - break; - } - } else if (IsAnyStore()) { - switch (lookup->state()) { - case LookupIterator::ACCESS_CHECK: - TRACE_HANDLER_STATS(isolate(), StoreIC_HandlerCacheHit_AccessCheck); - break; - case LookupIterator::INTEGER_INDEXED_EXOTIC: - TRACE_HANDLER_STATS(isolate(), StoreIC_HandlerCacheHit_Exotic); - break; - case LookupIterator::INTERCEPTOR: - TRACE_HANDLER_STATS(isolate(), StoreIC_HandlerCacheHit_Interceptor); - break; - case LookupIterator::JSPROXY: - TRACE_HANDLER_STATS(isolate(), StoreIC_HandlerCacheHit_JSProxy); - break; - case LookupIterator::NOT_FOUND: - TRACE_HANDLER_STATS(isolate(), StoreIC_HandlerCacheHit_NonExistent); - break; - case LookupIterator::ACCESSOR: - TRACE_HANDLER_STATS(isolate(), StoreIC_HandlerCacheHit_Accessor); - break; - case LookupIterator::DATA: - TRACE_HANDLER_STATS(isolate(), StoreIC_HandlerCacheHit_Data); - break; - case LookupIterator::TRANSITION: - TRACE_HANDLER_STATS(isolate(), StoreIC_HandlerCacheHit_Transition); - break; - } + TRACE_HANDLER_STATS(isolate(), LoadIC_HandlerCacheHit_Accessor); } else { - TRACE_HANDLER_STATS(isolate(), IC_HandlerCacheHit); + DCHECK(IsAnyStore()); + TRACE_HANDLER_STATS(isolate(), StoreIC_HandlerCacheHit_Accessor); } } -Handle IC::ComputeHandler(LookupIterator* lookup, - Handle value) { +Handle IC::ComputeHandler(LookupIterator* lookup) { // Try to find a globally shared handler stub. Handle shared_handler = GetMapIndependentHandler(lookup); if (!shared_handler.is_null()) { @@ -1087,7 +1035,7 @@ Handle IC::ComputeHandler(LookupIterator* lookup, return shared_handler; } - Handle handler = PropertyHandlerCompiler::Find( + Handle handler = PropertyHandlerCompiler::Find( lookup->name(), receiver_map(), handler_kind()); // Use the cached value if it exists, and if it is different from the // handler that just missed. @@ -1117,25 +1065,21 @@ Handle IC::ComputeHandler(LookupIterator* lookup, } } - handler = CompileHandler(lookup, value); - DCHECK(IC::IsHandler(*handler)); - if (handler->IsCode()) { - Handle code = Handle::cast(handler); - Map::UpdateCodeCache(receiver_map(), lookup->name(), code); - } + handler = CompileHandler(lookup); + Map::UpdateCodeCache(receiver_map(), lookup->name(), handler); return handler; } Handle LoadIC::GetMapIndependentHandler(LookupIterator* lookup) { Handle receiver = lookup->GetReceiver(); if (receiver->IsString() && - Name::Equals(isolate()->factory()->length_string(), lookup->name())) { + *lookup->name() == isolate()->heap()->length_string()) { FieldIndex index = FieldIndex::ForInObjectOffset(String::kLengthOffset); return SimpleFieldLoad(isolate(), index); } if (receiver->IsStringWrapper() && - Name::Equals(isolate()->factory()->length_string(), lookup->name())) { + *lookup->name() == isolate()->heap()->length_string()) { TRACE_HANDLER_STATS(isolate(), LoadIC_StringLengthStub); StringLengthStub string_length_stub(isolate()); return string_length_stub.GetCode(); @@ -1143,7 +1087,7 @@ Handle LoadIC::GetMapIndependentHandler(LookupIterator* lookup) { // Use specialized code for getting prototype of functions. if (receiver->IsJSFunction() && - Name::Equals(isolate()->factory()->prototype_string(), lookup->name()) && + *lookup->name() == isolate()->heap()->prototype_string() && receiver->IsConstructor() && !Handle::cast(receiver) ->map() @@ -1301,63 +1245,25 @@ Handle LoadIC::GetMapIndependentHandler(LookupIterator* lookup) { return Handle::null(); } -Handle LoadIC::CompileHandler(LookupIterator* lookup, - Handle unused) { +Handle LoadIC::CompileHandler(LookupIterator* lookup) { + DCHECK_EQ(LookupIterator::ACCESSOR, lookup->state()); Handle holder = lookup->GetHolder(); -#ifdef DEBUG - // Only used by DCHECKs below. - Handle receiver = lookup->GetReceiver(); - // Non-map-specific handler stubs have already been selected. - CHECK(!receiver->IsString() || - !Name::Equals(isolate()->factory()->length_string(), lookup->name())); - CHECK(!receiver->IsStringWrapper() || - !Name::Equals(isolate()->factory()->length_string(), lookup->name())); - - CHECK(!( - receiver->IsJSFunction() && - Name::Equals(isolate()->factory()->prototype_string(), lookup->name()) && - receiver->IsConstructor() && - !Handle::cast(receiver) - ->map() - ->has_non_instance_prototype())); -#endif - Handle map = receiver_map(); - switch (lookup->state()) { - case LookupIterator::ACCESSOR: { -#ifdef DEBUG - int object_offset; - DCHECK(!Accessors::IsJSObjectFieldAccessor(map, lookup->name(), - &object_offset)); -#endif - Handle accessors = lookup->GetAccessors(); - DCHECK(accessors->IsAccessorPair()); - DCHECK(holder->HasFastProperties()); - DCHECK(!GetHostFunction()->shared()->HasDebugInfo()); - Handle getter(Handle::cast(accessors)->getter(), - isolate()); - CallOptimization call_optimization(getter); - NamedLoadHandlerCompiler compiler(isolate(), map, holder); - DCHECK(call_optimization.is_simple_api_call()); - TRACE_HANDLER_STATS(isolate(), LoadIC_LoadCallback); - int index = lookup->GetAccessorIndex(); - Handle code = compiler.CompileLoadCallback( - lookup->name(), call_optimization, index, slow_stub()); - return code; - } - - case LookupIterator::INTERCEPTOR: - case LookupIterator::DATA: - case LookupIterator::INTEGER_INDEXED_EXOTIC: - case LookupIterator::ACCESS_CHECK: - case LookupIterator::JSPROXY: - case LookupIterator::NOT_FOUND: - case LookupIterator::TRANSITION: - UNREACHABLE(); - } - UNREACHABLE(); - return slow_stub(); + Handle accessors = lookup->GetAccessors(); + DCHECK(accessors->IsAccessorPair()); + DCHECK(holder->HasFastProperties()); + DCHECK(!GetHostFunction()->shared()->HasDebugInfo()); + Handle getter(Handle::cast(accessors)->getter(), + isolate()); + CallOptimization call_optimization(getter); + NamedLoadHandlerCompiler compiler(isolate(), map, holder); + DCHECK(call_optimization.is_simple_api_call()); + TRACE_HANDLER_STATS(isolate(), LoadIC_LoadCallback); + int index = lookup->GetAccessorIndex(); + Handle code = compiler.CompileLoadCallback( + lookup->name(), call_optimization, index, slow_stub()); + return code; } @@ -1652,7 +1558,7 @@ void StoreIC::UpdateCaches(LookupIterator* lookup, Handle value, Handle handler; if (LookupForWrite(lookup, value, store_mode)) { - handler = ComputeHandler(lookup, value); + handler = ComputeHandler(lookup); } else { TRACE_GENERIC_IC("LookupForWrite said 'false'"); handler = slow_stub(); @@ -1865,72 +1771,55 @@ Handle StoreIC::GetMapIndependentHandler(LookupIterator* lookup) { return Handle::null(); } -Handle StoreIC::CompileHandler(LookupIterator* lookup, - Handle value) { - DCHECK_NE(LookupIterator::JSPROXY, lookup->state()); +Handle StoreIC::CompileHandler(LookupIterator* lookup) { + DCHECK_EQ(LookupIterator::ACCESSOR, lookup->state()); // This is currently guaranteed by checks in StoreIC::Store. Handle receiver = Handle::cast(lookup->GetReceiver()); Handle holder = lookup->GetHolder(); DCHECK(!receiver->IsAccessCheckNeeded() || lookup->name()->IsPrivate()); - switch (lookup->state()) { - case LookupIterator::ACCESSOR: { - DCHECK(holder->HasFastProperties()); - Handle accessors = lookup->GetAccessors(); - if (accessors->IsAccessorInfo()) { - Handle info = Handle::cast(accessors); - DCHECK(v8::ToCData
(info->setter()) != 0); - DCHECK(!AccessorInfo::cast(*accessors)->is_special_data_property() || - lookup->HolderIsReceiverOrHiddenPrototype()); - DCHECK(AccessorInfo::IsCompatibleReceiverMap(isolate(), info, - receiver_map())); - TRACE_HANDLER_STATS(isolate(), StoreIC_StoreCallback); - NamedStoreHandlerCompiler compiler(isolate(), receiver_map(), holder); - // TODO(ishell): don't hard-code language mode into the handler because - // this handler can be re-used through megamorphic stub cache for wrong - // language mode. - // Better pass vector/slot to Runtime::kStoreCallbackProperty and - // let it decode the language mode from the IC kind. - Handle code = compiler.CompileStoreCallback( - receiver, lookup->name(), info, language_mode()); - return code; - } else { - DCHECK(accessors->IsAccessorPair()); - Handle setter(Handle::cast(accessors)->setter(), - isolate()); - DCHECK(setter->IsJSFunction() || setter->IsFunctionTemplateInfo()); - CallOptimization call_optimization(setter); - NamedStoreHandlerCompiler compiler(isolate(), receiver_map(), holder); - if (call_optimization.is_simple_api_call()) { - DCHECK(call_optimization.IsCompatibleReceiver(receiver, holder)); - TRACE_HANDLER_STATS(isolate(), StoreIC_StoreCallback); - Handle code = compiler.CompileStoreCallback( - receiver, lookup->name(), call_optimization, - lookup->GetAccessorIndex(), slow_stub()); - return code; - } - TRACE_HANDLER_STATS(isolate(), StoreIC_StoreViaSetter); - int expected_arguments = JSFunction::cast(*setter) - ->shared() - ->internal_formal_parameter_count(); - return compiler.CompileStoreViaSetter(receiver, lookup->name(), - lookup->GetAccessorIndex(), - expected_arguments); - } - } + DCHECK(holder->HasFastProperties()); + Handle accessors = lookup->GetAccessors(); - case LookupIterator::DATA: - case LookupIterator::TRANSITION: - case LookupIterator::INTERCEPTOR: - case LookupIterator::INTEGER_INDEXED_EXOTIC: - case LookupIterator::ACCESS_CHECK: - case LookupIterator::JSPROXY: - case LookupIterator::NOT_FOUND: - UNREACHABLE(); + if (accessors->IsAccessorInfo()) { + Handle info = Handle::cast(accessors); + DCHECK(v8::ToCData
(info->setter()) != 0); + DCHECK(!AccessorInfo::cast(*accessors)->is_special_data_property() || + lookup->HolderIsReceiverOrHiddenPrototype()); + DCHECK( + AccessorInfo::IsCompatibleReceiverMap(isolate(), info, receiver_map())); + TRACE_HANDLER_STATS(isolate(), StoreIC_StoreCallback); + NamedStoreHandlerCompiler compiler(isolate(), receiver_map(), holder); + // TODO(ishell): don't hard-code language mode into the handler because + // this handler can be re-used through megamorphic stub cache for wrong + // language mode. + // Better pass vector/slot to Runtime::kStoreCallbackProperty and + // let it decode the language mode from the IC kind. + Handle code = compiler.CompileStoreCallback(receiver, lookup->name(), + info, language_mode()); + return code; } - UNREACHABLE(); - return slow_stub(); + + DCHECK(accessors->IsAccessorPair()); + Handle setter(Handle::cast(accessors)->setter(), + isolate()); + DCHECK(setter->IsJSFunction() || setter->IsFunctionTemplateInfo()); + CallOptimization call_optimization(setter); + NamedStoreHandlerCompiler compiler(isolate(), receiver_map(), holder); + if (call_optimization.is_simple_api_call()) { + DCHECK(call_optimization.IsCompatibleReceiver(receiver, holder)); + TRACE_HANDLER_STATS(isolate(), StoreIC_StoreCallback); + Handle code = compiler.CompileStoreCallback( + receiver, lookup->name(), call_optimization, lookup->GetAccessorIndex(), + slow_stub()); + return code; + } + TRACE_HANDLER_STATS(isolate(), StoreIC_StoreViaSetter); + int expected_arguments = + JSFunction::cast(*setter)->shared()->internal_formal_parameter_count(); + return compiler.CompileStoreViaSetter( + receiver, lookup->name(), lookup->GetAccessorIndex(), expected_arguments); } void KeyedStoreIC::UpdateStoreElement(Handle receiver_map, diff --git a/src/ic/ic.h b/src/ic/ic.h index 8b18753f25..0a8f665f94 100644 --- a/src/ic/ic.h +++ b/src/ic/ic.h @@ -117,16 +117,14 @@ class IC { void TraceHandlerCacheHitStats(LookupIterator* lookup); // Compute the handler either by compiling or by retrieving a cached version. - Handle ComputeHandler(LookupIterator* lookup, - Handle value = Handle::null()); + Handle ComputeHandler(LookupIterator* lookup); virtual Handle GetMapIndependentHandler(LookupIterator* lookup) { UNREACHABLE(); return Handle::null(); } - virtual Handle CompileHandler(LookupIterator* lookup, - Handle value) { + virtual Handle CompileHandler(LookupIterator* lookup) { UNREACHABLE(); - return Handle::null(); + return Handle::null(); } void UpdateMonomorphicIC(Handle handler, Handle name); @@ -273,8 +271,7 @@ class LoadIC : public IC { Handle GetMapIndependentHandler(LookupIterator* lookup) override; - Handle CompileHandler(LookupIterator* lookup, - Handle unused) override; + Handle CompileHandler(LookupIterator* lookup) override; private: // Creates a data handler that represents a load of a field by given index. @@ -360,8 +357,7 @@ class StoreIC : public IC { void UpdateCaches(LookupIterator* lookup, Handle value, JSReceiver::StoreFromKeyed store_mode); Handle GetMapIndependentHandler(LookupIterator* lookup) override; - Handle CompileHandler(LookupIterator* lookup, - Handle value) override; + Handle CompileHandler(LookupIterator* lookup) override; private: Handle StoreTransition(Handle receiver_map,