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

diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp
index aec04b4..b24c211 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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, " <Token>%u</Token> ", &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, " <Hash>%u</Hash> ", &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, " <Offset>%u</Offset> ", &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 (file)
--- 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)