From 7b9e24e3fba995c6a5143a90efcf47f791579f2f Mon Sep 17 00:00:00 2001 From: "mvstanton@chromium.org" Date: Thu, 7 Aug 2014 15:33:14 +0000 Subject: [PATCH] Clean up IC tracing for CallICs. CallICs have had some confused tracing, because the IC state is not entirely captured by the installed code stub - it lives in the type vector. Change tracing to be able to use the vector state changes instead. Introduced a DEFAULT state to be used by all vector-based ICs. R=verwaest@chromium.org BUG= Review URL: https://codereview.chromium.org/451643002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@22978 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/code-stubs.cc | 2 +- src/code-stubs.h | 26 ++++++++++----------- src/globals.h | 6 ++++- src/ic-inl.h | 4 +++- src/ic.cc | 69 +++++++++++++++++++++++++++++++++++-------------------- src/ic.h | 11 +++++---- src/objects.cc | 2 ++ src/objects.h | 9 ++++---- 8 files changed, 78 insertions(+), 51 deletions(-) diff --git a/src/code-stubs.cc b/src/code-stubs.cc index 88e3cc9..0e68ab8 100644 --- a/src/code-stubs.cc +++ b/src/code-stubs.cc @@ -320,7 +320,7 @@ void StringAddStub::PrintBaseName(OStream& os) const { // NOLINT } -InlineCacheState ICCompareStub::GetICState() { +InlineCacheState ICCompareStub::GetICState() const { CompareIC::State state = Max(left_, right_); switch (state) { case CompareIC::UNINITIALIZED: diff --git a/src/code-stubs.h b/src/code-stubs.h index 6663b25..1e1d3cc 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -183,9 +183,7 @@ class CodeStub BASE_EMBEDDED { virtual Major MajorKey() const = 0; virtual int MinorKey() const = 0; - virtual InlineCacheState GetICState() { - return UNINITIALIZED; - } + virtual InlineCacheState GetICState() const { return UNINITIALIZED; } virtual ExtraICState GetExtraICState() const { return kNoExtraICState; } virtual Code::StubType GetStubType() { return Code::NORMAL; @@ -850,9 +848,7 @@ class CallICStub: public PlatformCodeStub { return Code::CALL_IC; } - virtual InlineCacheState GetICState() V8_FINAL V8_OVERRIDE { - return state_.GetICState(); - } + virtual InlineCacheState GetICState() const V8_OVERRIDE { return DEFAULT; } virtual ExtraICState GetExtraICState() const V8_FINAL V8_OVERRIDE { return state_.GetExtraICState(); @@ -878,6 +874,10 @@ class CallIC_ArrayStub: public CallICStub { virtual void Generate(MacroAssembler* masm); + virtual InlineCacheState GetICState() const V8_FINAL V8_OVERRIDE { + return MONOMORPHIC; + } + protected: virtual void PrintState(OStream& os) const V8_OVERRIDE; // NOLINT @@ -903,7 +903,7 @@ class HandlerStub : public HydrogenCodeStub { public: virtual Code::Kind GetCodeKind() const { return Code::HANDLER; } virtual ExtraICState GetExtraICState() const { return kind(); } - virtual InlineCacheState GetICState() { return MONOMORPHIC; } + virtual InlineCacheState GetICState() const { return MONOMORPHIC; } virtual void InitializeInterfaceDescriptor( CodeStubInterfaceDescriptor* descriptor) V8_OVERRIDE; @@ -1126,7 +1126,7 @@ class BinaryOpICStub : public HydrogenCodeStub { return Code::BINARY_OP_IC; } - virtual InlineCacheState GetICState() V8_FINAL V8_OVERRIDE { + virtual InlineCacheState GetICState() const V8_FINAL V8_OVERRIDE { return state_.GetICState(); } @@ -1179,7 +1179,7 @@ class BinaryOpICWithAllocationSiteStub V8_FINAL : public PlatformCodeStub { return Code::BINARY_OP_IC; } - virtual InlineCacheState GetICState() V8_OVERRIDE { + virtual InlineCacheState GetICState() const V8_OVERRIDE { return state_.GetICState(); } @@ -1316,7 +1316,7 @@ class ICCompareStub: public PlatformCodeStub { CompareIC::State* right_state, CompareIC::State* handler_state, Token::Value* op); - virtual InlineCacheState GetICState(); + virtual InlineCacheState GetICState() const; private: class OpField: public BitField { }; @@ -1384,7 +1384,7 @@ class CompareNilICStub : public HydrogenCodeStub { isolate->code_stub_interface_descriptor(CodeStub::CompareNilIC)); } - virtual InlineCacheState GetICState() { + virtual InlineCacheState GetICState() const { if (state_.Contains(GENERIC)) { return MEGAMORPHIC; } else if (state_.Contains(MONOMORPHIC_MAP)) { @@ -1875,7 +1875,7 @@ class KeyedLoadGenericStub : public HydrogenCodeStub { static void InstallDescriptors(Isolate* isolate); virtual Code::Kind GetCodeKind() const { return Code::KEYED_LOAD_IC; } - virtual InlineCacheState GetICState() { return GENERIC; } + virtual InlineCacheState GetICState() const { return GENERIC; } private: Major MajorKey() const { return KeyedLoadGeneric; } @@ -2362,7 +2362,7 @@ class ToBooleanStub: public HydrogenCodeStub { virtual ExtraICState GetExtraICState() const { return types_.ToIntegral(); } - virtual InlineCacheState GetICState() { + virtual InlineCacheState GetICState() const { if (types_.IsEmpty()) { return ::v8::internal::UNINITIALIZED; } else { diff --git a/src/globals.h b/src/globals.h index 000fa47..889822f 100644 --- a/src/globals.h +++ b/src/globals.h @@ -446,7 +446,11 @@ enum InlineCacheState { // A generic handler is installed and no extra typefeedback is recorded. GENERIC, // Special state for debug break or step in prepare stubs. - DEBUG_STUB + DEBUG_STUB, + // Type-vector-based ICs have a default state, with the full calculation + // of IC state only determined by a look at the IC and the typevector + // together. + DEFAULT }; diff --git a/src/ic-inl.h b/src/ic-inl.h index ad0078e..c7954ce 100644 --- a/src/ic-inl.h +++ b/src/ic-inl.h @@ -169,8 +169,10 @@ Handle IC::GetICCacheHolder(HeapType* type, Isolate* isolate, } -IC::State CallIC::FeedbackObjectToState(Object* feedback) const { +IC::State CallIC::FeedbackToState(Handle vector, + Handle slot) const { IC::State state = UNINITIALIZED; + Object* feedback = vector->get(slot->value()); if (feedback == *TypeFeedbackInfo::MegamorphicSentinel(isolate())) { state = GENERIC; diff --git a/src/ic.cc b/src/ic.cc index 90d77f2..2139d5c 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -33,6 +33,9 @@ char IC::TransitionMarkFromState(IC::State state) { // computed from the original code - not the patched code. Let // these cases fall through to the unreachable code below. case DEBUG_STUB: break; + // Type-vector-based ICs resolve state to one of the above. + case DEFAULT: + break; } UNREACHABLE(); return 0; @@ -66,10 +69,20 @@ const char* GetTransitionMarkModifier(KeyedAccessStoreMode mode) { #endif // DEBUG + void IC::TraceIC(const char* type, Handle name) { if (FLAG_trace_ic) { Code* new_target = raw_target(); State new_state = new_target->ic_state(); + TraceIC(type, name, state(), new_state); + } +} + + +void IC::TraceIC(const char* type, Handle name, State old_state, + State new_state) { + if (FLAG_trace_ic) { + Code* new_target = raw_target(); PrintF("[%s%s in ", new_target->is_keyed_stub() ? "Keyed" : "", type); // TODO(jkummerow): Add support for "apply". The logic is roughly: @@ -92,10 +105,8 @@ void IC::TraceIC(const char* type, Handle name) { modifier = GetTransitionMarkModifier( KeyedStoreIC::GetKeyedAccessStoreMode(extra_state)); } - PrintF(" (%c->%c%s)", - TransitionMarkFromState(state()), - TransitionMarkFromState(new_state), - modifier); + PrintF(" (%c->%c%s)", TransitionMarkFromState(old_state), + TransitionMarkFromState(new_state), modifier); #ifdef OBJECT_PRINT OFStream os(stdout); name->Print(os); @@ -107,7 +118,8 @@ void IC::TraceIC(const char* type, Handle name) { } #define TRACE_IC(type, name) TraceIC(type, name) - +#define TRACE_VECTOR_IC(type, name, old_state, new_state) \ + TraceIC(type, name, old_state, new_state) IC::IC(FrameDepth depth, Isolate* isolate) : isolate_(isolate), @@ -365,6 +377,7 @@ static void ComputeTypeInfoCountDelta(IC::State old_state, IC::State new_state, break; case PROTOTYPE_FAILURE: case DEBUG_STUB: + case DEFAULT: UNREACHABLE(); } } @@ -807,6 +820,7 @@ void IC::PatchCache(Handle name, Handle code) { break; case DEBUG_STUB: break; + case DEFAULT: case GENERIC: UNREACHABLE(); break; @@ -1950,6 +1964,7 @@ bool CallIC::DoCustomHandler(Handle receiver, isolate()->native_context()->array_function()); if (array_function.is_identical_to(Handle::cast(function))) { // Alter the slot. + IC::State old_state = FeedbackToState(vector, slot); Object* feedback = vector->get(slot->value()); if (!feedback->IsAllocationSite()) { Handle new_site = @@ -1965,18 +1980,19 @@ bool CallIC::DoCustomHandler(Handle receiver, isolate()); } - TRACE_IC("CallIC (Array call)", name); - Object* new_feedback = vector->get(slot->value()); - UpdateTypeFeedbackInfo(feedback, new_feedback); + IC::State new_state = FeedbackToState(vector, slot); + OnTypeFeedbackChanged(isolate(), address(), old_state, new_state, true); + TRACE_VECTOR_IC("CallIC (custom handler)", name, old_state, new_state); return true; } return false; } -void CallIC::PatchMegamorphic(Handle vector, - Handle slot) { +void CallIC::PatchMegamorphic(Handle function, + Handle vector, Handle slot) { State state(target()->extra_ic_state()); + IC::State old_state = FeedbackToState(vector, slot); // We are going generic. vector->set(slot->value(), @@ -1987,15 +2003,15 @@ void CallIC::PatchMegamorphic(Handle vector, Handle code = stub.GetCode(); set_target(*code); - TRACE_GENERIC_IC(isolate(), "CallIC", "megamorphic"); -} - + Handle name = isolate()->factory()->empty_string(); + if (function->IsJSFunction()) { + Handle js_function = Handle::cast(function); + name = handle(js_function->shared()->name(), isolate()); + } -void CallIC::UpdateTypeFeedbackInfo(Object* old_feedback, - Object* new_feedback) { - IC::State old_state = FeedbackObjectToState(old_feedback); - IC::State new_state = FeedbackObjectToState(new_feedback); + IC::State new_state = FeedbackToState(vector, slot); OnTypeFeedbackChanged(isolate(), address(), old_state, new_state, true); + TRACE_VECTOR_IC("CallIC", name, old_state, new_state); } @@ -2004,6 +2020,8 @@ void CallIC::HandleMiss(Handle receiver, Handle vector, Handle slot) { State state(target()->extra_ic_state()); + IC::State old_state = FeedbackToState(vector, slot); + Handle name = isolate()->factory()->empty_string(); Object* feedback = vector->get(slot->value()); // Hand-coded MISS handling is easier if CallIC slots don't contain smis. @@ -2014,8 +2032,6 @@ void CallIC::HandleMiss(Handle receiver, vector->set(slot->value(), *TypeFeedbackInfo::MegamorphicSentinel(isolate()), SKIP_WRITE_BARRIER); - - TRACE_GENERIC_IC(isolate(), "CallIC", "megamorphic"); } else { // The feedback is either uninitialized or an allocation site. // It might be an allocation site because if we re-compile the full code @@ -2032,14 +2048,17 @@ void CallIC::HandleMiss(Handle receiver, return; } - Handle js_function = Handle::cast(function); - Handle name(js_function->shared()->name(), isolate()); - TRACE_IC("CallIC", name); vector->set(slot->value(), *function); } - Object* new_feedback = vector->get(slot->value()); - UpdateTypeFeedbackInfo(feedback, new_feedback); + if (function->IsJSFunction()) { + Handle js_function = Handle::cast(function); + name = handle(js_function->shared()->name(), isolate()); + } + + IC::State new_state = FeedbackToState(vector, slot); + OnTypeFeedbackChanged(isolate(), address(), old_state, new_state, true); + TRACE_VECTOR_IC("CallIC", name, old_state, new_state); } @@ -2074,7 +2093,7 @@ RUNTIME_FUNCTION(CallIC_Customization_Miss) { Handle function = args.at(1); Handle vector = args.at(2); Handle slot = args.at(3); - ic.PatchMegamorphic(vector, slot); + ic.PatchMegamorphic(function, vector, slot); return *function; } diff --git a/src/ic.h b/src/ic.h index 72cc3f6..9877168 100644 --- a/src/ic.h +++ b/src/ic.h @@ -163,6 +163,8 @@ class IC { char TransitionMarkFromState(IC::State state); void TraceIC(const char* type, Handle name); + void TraceIC(const char* type, Handle name, State old_state, + State new_state); MaybeHandle TypeError(const char* type, Handle object, @@ -335,8 +337,6 @@ class CallIC: public IC { : argc_(argc), call_type_(call_type) { } - InlineCacheState GetICState() const { return ::v8::internal::GENERIC; } - ExtraICState GetExtraICState() const; static void GenerateAheadOfTime( @@ -359,7 +359,8 @@ class CallIC: public IC { : IC(EXTRA_CALL_FRAME, isolate) { } - void PatchMegamorphic(Handle vector, Handle slot); + void PatchMegamorphic(Handle function, Handle vector, + Handle slot); void HandleMiss(Handle receiver, Handle function, @@ -382,8 +383,8 @@ class CallIC: public IC { ConstantPoolArray* constant_pool); private: - void UpdateTypeFeedbackInfo(Object* old_feedback, Object* new_feedback); - inline IC::State FeedbackObjectToState(Object* feedback) const; + inline IC::State FeedbackToState(Handle vector, + Handle slot) const; }; diff --git a/src/objects.cc b/src/objects.cc index abf135b..2262e57 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -11320,6 +11320,8 @@ const char* Code::ICState2String(InlineCacheState state) { case MEGAMORPHIC: return "MEGAMORPHIC"; case GENERIC: return "GENERIC"; case DEBUG_STUB: return "DEBUG_STUB"; + case DEFAULT: + return "DEFAULT"; } UNREACHABLE(); return NULL; diff --git a/src/objects.h b/src/objects.h index 43c92a4..eb4af1d 100644 --- a/src/objects.h +++ b/src/objects.h @@ -5878,11 +5878,10 @@ class Code: public HeapObject { static const int kProfilerTicksOffset = kFullCodeFlags + 1; // Flags layout. BitField. - class ICStateField: public BitField {}; - class TypeField: public BitField {}; - class CacheHolderField : public BitField {}; - class KindField: public BitField {}; - // TODO(bmeurer): Bit 10 is available for free use. :-) + class ICStateField : public BitField {}; + class TypeField : public BitField {}; + class CacheHolderField : public BitField {}; + class KindField : public BitField {}; class ExtraICStateField: public BitField {}; // NOLINT -- 2.7.4