From 6d0e7f2f49b30f69d0f5e34564b4f810aa1055c1 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 24 May 2016 11:12:14 -0700 Subject: [PATCH] Inliner: use offset in xml replay Replay needs the ability to distinguish among multiple calls to the same callee. The IL offset of the call site can be used for this, so start checking for it during replay. Note there is subsequent follow-up work to ensure that we actually track offsets for calls. Refactor the way the ReplayPolicy gets notified of the current inline context (and the IL offset of the call). Instead of trying to pass this information as arguments to the factory method that creates policies, add special notifications for this information that only the ReplayPolicy will act upon. Commit migrated from https://github.com/dotnet/coreclr/commit/be9d889b084099797dfcf019b2820433558dbec9 --- src/coreclr/src/jit/flowgraph.cpp | 2 +- src/coreclr/src/jit/inline.cpp | 36 +++++++++++++++++++----- src/coreclr/src/jit/inline.h | 14 ++++++++-- src/coreclr/src/jit/inlinepolicy.cpp | 54 +++++++++++++++++++++++++----------- src/coreclr/src/jit/inlinepolicy.h | 16 +++++++++-- 5 files changed, 94 insertions(+), 28 deletions(-) diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp index aec04b4..b24c211 100644 --- a/src/coreclr/src/jit/flowgraph.cpp +++ b/src/coreclr/src/jit/flowgraph.cpp @@ -21481,7 +21481,7 @@ void Compiler::fgInline() if ((expr->gtOper == GT_CALL) && ((expr->gtFlags & GTF_CALL_INLINE_CANDIDATE) != 0)) { GenTreeCall* call = expr->AsCall(); - InlineResult inlineResult(this, call, stmt->gtInlineContext, "fgInline"); + InlineResult inlineResult(this, call, stmt, "fgInline"); fgMorphStmt = stmt; diff --git a/src/coreclr/src/jit/inline.cpp b/src/coreclr/src/jit/inline.cpp index f9feeec..d77e2e2 100644 --- a/src/coreclr/src/jit/inline.cpp +++ b/src/coreclr/src/jit/inline.cpp @@ -443,7 +443,7 @@ void InlineContext::DumpData(unsigned indent) if (m_Parent == nullptr) { // Root method... cons up a policy so we can display the name - InlinePolicy* policy = InlinePolicy::GetPolicy(compiler, nullptr, true); + InlinePolicy* policy = InlinePolicy::GetPolicy(compiler, true); printf("\nInlines [%u] into \"%s\" [%s]\n", m_InlineStrategy->GetInlineCount(), calleeName, @@ -549,17 +549,17 @@ void InlineContext::DumpXml(FILE* file, unsigned indent) // Arguments: // compiler - the compiler instance examining a call for inlining // call - the call in question -// inlineContext - the inline context for the inline, if known +// stmt - statement containing the call (if known) // description - string describing the context of the decision InlineResult::InlineResult(Compiler* compiler, GenTreeCall* call, - InlineContext* inlineContext, + GenTreeStmt* stmt, const char* description) : m_RootCompiler(nullptr) , m_Policy(nullptr) , m_Call(call) - , m_InlineContext(inlineContext) + , m_InlineContext(nullptr) , m_Caller(nullptr) , m_Callee(nullptr) , m_Description(description) @@ -570,7 +570,20 @@ InlineResult::InlineResult(Compiler* compiler, // Set the policy const bool isPrejitRoot = false; - m_Policy = InlinePolicy::GetPolicy(m_RootCompiler, m_InlineContext, isPrejitRoot); + m_Policy = InlinePolicy::GetPolicy(m_RootCompiler, isPrejitRoot); + + // Pass along some optional information to the policy. + if (stmt != nullptr) + { + m_InlineContext = stmt->gtInlineContext; + m_Policy->NoteContext(m_InlineContext); + +#if defined(DEBUG) || defined(INLINE_DATA) + m_Policy->NoteOffset(call->gtRawILOffset); +#else + m_Policy->NoteOffset(stmt->gtStmtILoffsx); +#endif // defined(DEBUG) || defined(INLINE_DATA) + } // Get method handle for caller. Note we use the // handle for the "immediate" caller here. @@ -616,7 +629,7 @@ InlineResult::InlineResult(Compiler* compiler, // Set the policy const bool isPrejitRoot = true; - m_Policy = InlinePolicy::GetPolicy(m_RootCompiler, nullptr, isPrejitRoot); + m_Policy = InlinePolicy::GetPolicy(m_RootCompiler, isPrejitRoot); } //------------------------------------------------------------------------ @@ -1095,6 +1108,8 @@ InlineContext* InlineStrategy::NewSuccess(InlineInfo* inlineInfo) calleeContext->m_Callee = inlineInfo->fncHandle; // +1 here since we set this before calling NoteOutcome. calleeContext->m_Ordinal = m_InlineCount + 1; + // Update offset with more accurate info + calleeContext->m_Offset = inlineInfo->inlineResult->GetCall()->gtRawILOffset; #endif // defined(DEBUG) || defined(INLINE_DATA) @@ -1157,6 +1172,13 @@ InlineContext* InlineStrategy::NewFailure(GenTree* stmt, failedContext->m_Callee = inlineResult->GetCallee(); failedContext->m_Success = false; +#if defined(DEBUG) || defined(INLINE_DATA) + + // Update offset with more accurate info + failedContext->m_Offset = inlineResult->GetCall()->gtRawILOffset; + +#endif // #if defined(DEBUG) || defined(INLINE_DATA) + #if defined(DEBUG) failedContext->m_TreeID = inlineResult->GetCall()->gtTreeID; @@ -1266,7 +1288,7 @@ void InlineStrategy::DumpDataEnsurePolicyIsSet() if (m_LastSuccessfulPolicy == nullptr) { const bool isPrejitRoot = (opts.eeFlags & CORJIT_FLG_PREJIT) != 0; - m_LastSuccessfulPolicy = InlinePolicy::GetPolicy(m_Compiler, nullptr, isPrejitRoot); + m_LastSuccessfulPolicy = InlinePolicy::GetPolicy(m_Compiler, isPrejitRoot); // Add in a bit of data.... const bool isForceInline = (info.compFlags & CORINFO_FLG_FORCEINLINE) != 0; diff --git a/src/coreclr/src/jit/inline.h b/src/coreclr/src/jit/inline.h index d25152b..effe421 100644 --- a/src/coreclr/src/jit/inline.h +++ b/src/coreclr/src/jit/inline.h @@ -218,7 +218,7 @@ class InlinePolicy public: // Factory method for getting policies - static InlinePolicy* GetPolicy(Compiler* compiler, InlineContext* context, bool isPrejitRoot); + static InlinePolicy* GetPolicy(Compiler* compiler, bool isPrejitRoot); // Obligatory virtual dtor virtual ~InlinePolicy() {} @@ -235,6 +235,10 @@ public: virtual void NoteFatal(InlineObservation obs) = 0; virtual void NoteInt(InlineObservation obs, int value) = 0; + // Optional observations. Most policies ignore these. + virtual void NoteContext(InlineContext* context) { (void) context; } + virtual void NoteOffset(IL_OFFSETX offset) { (void) offset; } + // Policy determinations virtual void DetermineProfitability(CORINFO_METHOD_INFO* methodInfo) = 0; @@ -289,7 +293,7 @@ public: // particular call for inlining. InlineResult(Compiler* compiler, GenTreeCall* call, - InlineContext* inlineContext, + GenTreeStmt* stmt, const char* description); // Construct a new InlineResult to evaluate a particular @@ -622,6 +626,12 @@ public: return m_CodeSizeEstimate; } + // Get the offset of the call site + IL_OFFSETX GetOffset() const + { + return m_Offset; + } + // True if this is the root context bool IsRoot() const { diff --git a/src/coreclr/src/jit/inlinepolicy.cpp b/src/coreclr/src/jit/inlinepolicy.cpp index 332ffe0..0f1bd0c 100644 --- a/src/coreclr/src/jit/inlinepolicy.cpp +++ b/src/coreclr/src/jit/inlinepolicy.cpp @@ -15,7 +15,6 @@ // // Arguments: // compiler - the compiler instance that will evaluate inlines -// inlineContext - the context of the inline // isPrejitRoot - true if this policy is evaluating a prejit root // // Return Value: @@ -25,12 +24,9 @@ // Determines which of the various policies should apply, // and creates (or reuses) a policy instance to use. -InlinePolicy* InlinePolicy::GetPolicy(Compiler* compiler, InlineContext* inlineContext, bool isPrejitRoot) +InlinePolicy* InlinePolicy::GetPolicy(Compiler* compiler, bool isPrejitRoot) { - // inlineContext only conditionally used below. - (void) inlineContext; - #ifdef DEBUG // Optionally install the RandomPolicy. @@ -52,7 +48,7 @@ InlinePolicy* InlinePolicy::GetPolicy(Compiler* compiler, InlineContext* inlineC if (useReplayPolicy) { - return new (compiler, CMK_Inlining) ReplayPolicy(compiler, inlineContext, isPrejitRoot); + return new (compiler, CMK_Inlining) ReplayPolicy(compiler, isPrejitRoot); } // Optionally install the SizePolicy. @@ -2078,12 +2074,11 @@ CritSecObject ReplayPolicy::s_XmlReaderLock; // // Arguments: // compiler -- compiler instance doing the inlining (root compiler) -// inlineContext -- inline context for the inline // isPrejitRoot -- true if this compiler is prejitting the root method -ReplayPolicy::ReplayPolicy(Compiler* compiler, InlineContext* inlineContext, bool isPrejitRoot) +ReplayPolicy::ReplayPolicy(Compiler* compiler, bool isPrejitRoot) : DiscretionaryPolicy(compiler, isPrejitRoot) - , m_InlineContext(inlineContext) + , m_InlineContext(nullptr) { // Is there a log file open already? If so, we can use it. if (s_ReplayFile == nullptr) @@ -2266,8 +2261,9 @@ bool ReplayPolicy::FindContext(InlineContext* context) unsigned contextHash = m_RootCompiler->info.compCompHnd->getMethodHash( context->GetCallee()); + unsigned contextOffset = (unsigned) context->GetOffset(); - return FindInline(contextToken, contextHash); + return FindInline(contextToken, contextHash, contextOffset); } //------------------------------------------------------------------------ @@ -2276,6 +2272,7 @@ bool ReplayPolicy::FindContext(InlineContext* context) // Arguments: // token -- token describing the inline // hash -- hash describing the inline +// offset -- IL offset of the call site in the parent method // // ReturnValue: // true if the inline entry was found @@ -2288,7 +2285,7 @@ bool ReplayPolicy::FindContext(InlineContext* context) // particular inline, if there are multiple calls to the same // method. -bool ReplayPolicy::FindInline(unsigned token, unsigned hash) +bool ReplayPolicy::FindInline(unsigned token, unsigned hash, unsigned offset) { char buffer[256]; bool foundInline = false; @@ -2361,11 +2358,10 @@ bool ReplayPolicy::FindInline(unsigned token, unsigned hash) break; } + // Match token unsigned inlineToken = 0; int count = sscanf(buffer, " %u ", &inlineToken); - // Need a secondary check here for callsite. - // ...offset or similar. if ((count != 1) || (inlineToken != token)) { continue; @@ -2377,16 +2373,32 @@ bool ReplayPolicy::FindInline(unsigned token, unsigned hash) break; } + // Match hash unsigned inlineHash = 0; count = sscanf(buffer, " %u ", &inlineHash); - // Need a secondary check here for callsite ID - // ... offset or similar. if ((count != 1) || (inlineHash != hash)) { continue; } + // Get next line + if (fgets(buffer, sizeof(buffer), s_ReplayFile) == nullptr) + { + break; + } + + // Match offset + unsigned inlineOffset = 0; + count = sscanf(buffer, " %u ", &inlineOffset); + if ((count != 1) || (inlineOffset != offset)) + { + continue; + } + + // Token,Hash,Offset may still not be unique enough, but it's + // all we have right now. + // We're good! foundInline = true; break; @@ -2420,7 +2432,17 @@ bool ReplayPolicy::FindInline(CORINFO_METHOD_HANDLE callee) unsigned calleeHash = m_RootCompiler->info.compCompHnd->getMethodHash(callee); - bool foundInline = FindInline(calleeToken, calleeHash); + // Abstract this or just pass through raw bits + // See matching code in xml writer + int offset = -1; + if (m_Offset != BAD_IL_OFFSET) + { + offset = (int) jitGetILoffs(m_Offset); + } + + unsigned calleeOffset = (unsigned) offset; + + bool foundInline = FindInline(calleeToken, calleeHash, calleeOffset); return foundInline; } diff --git a/src/coreclr/src/jit/inlinepolicy.h b/src/coreclr/src/jit/inlinepolicy.h index 21bcf51..4b71106 100644 --- a/src/coreclr/src/jit/inlinepolicy.h +++ b/src/coreclr/src/jit/inlinepolicy.h @@ -346,7 +346,18 @@ class ReplayPolicy : public DiscretionaryPolicy public: // Construct a ReplayPolicy - ReplayPolicy(Compiler* compiler, InlineContext* inlineContext, bool isPrejitRoot); + ReplayPolicy(Compiler* compiler, bool isPrejitRoot); + + // Optional observations + void NoteContext(InlineContext* context) override + { + m_InlineContext = context; + } + + void NoteOffset(IL_OFFSETX offset) override + { + m_Offset = offset; + } // Policy determinations void DetermineProfitability(CORINFO_METHOD_INFO* methodInfo) override; @@ -361,12 +372,13 @@ private: bool FindMethod(); bool FindContext(InlineContext* context); bool FindInline(CORINFO_METHOD_HANDLE callee); - bool FindInline(unsigned token, unsigned hash); + bool FindInline(unsigned token, unsigned hash, unsigned offset); static bool s_WroteReplayBanner; static FILE* s_ReplayFile; static CritSecObject s_XmlReaderLock; InlineContext* m_InlineContext; + IL_OFFSETX m_Offset; }; #endif // defined(DEBUG) || defined(INLINE_DATA) -- 2.7.4