Fixes for text-based PGO (#56986)

Fix missing indirection when reading in text-based PGO data.

Prioritize reading text-based PGO over dynamic or static PGO data, so that we
can use text to provide a fixed set of PGO data when trying to isolate bugs.

In the jit, give text-based PGO data the same trust level we give to dynamic
PGO data.

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index 06f3763..f843a62 100644 (file)
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -5849,6 +5849,7 @@ public:
     void fgSetProfileWeight(BasicBlock* block, BasicBlock::weight_t weight);
     void fgApplyProfileScale();
     bool fgHaveSufficientProfileData();
+    bool fgHaveTrustedProfileData();
 
     // fgIsUsingProfileWeights - returns true if we have real profile data for this method
     //                           or if we have some fake profile data for the stress mode

diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp
index d5667a2..47019dc 100644 (file)
--- a/src/coreclr/jit/fgprofile.cpp
+++ b/src/coreclr/jit/fgprofile.cpp
@@ -73,6 +73,37 @@ bool Compiler::fgHaveSufficientProfileData()
 }
 
 //------------------------------------------------------------------------
+// fgHaveTrustedProfileData: check if profile data source is one
+//   that can be trusted to faithfully represent the current program
+//   behavior.
+//
+// Returns:
+//   true if so
+//
+// Note:
+//   See notes for fgHaveProfileData.
+//
+bool Compiler::fgHaveTrustedProfileData()
+{
+    if (!fgHaveProfileData())
+    {
+        return false;
+    }
+
+    // We allow Text to be trusted so we can use it to stand in
+    // for Dynamic results.
+    //
+    switch (fgPgoSource)
+    {
+        case ICorJitInfo::PgoSource::Dynamic:
+        case ICorJitInfo::PgoSource::Text:
+            return true;
+        default:
+            return false;
+    }
+}
+
+//------------------------------------------------------------------------
 // fgApplyProfileScale: scale inlinee counts by appropriate scale factor
 //
 void Compiler::fgApplyProfileScale()
@@ -1796,6 +1827,9 @@ PhaseStatus Compiler::fgIncorporateProfileData()
                 break;
 
             default:
+                JITDUMP("Unknown PGO record type 0x%x in schema entry %u (offset 0x%x count 0x%x other 0x%x)\n",
+                        fgPgoSchema[iSchema].InstrumentationKind, iSchema, fgPgoSchema[iSchema].ILOffset,
+                        fgPgoSchema[iSchema].Count, fgPgoSchema[iSchema].Other);
                 otherRecords++;
                 break;
         }

diff --git a/src/coreclr/jit/inlinepolicy.cpp b/src/coreclr/jit/inlinepolicy.cpp
index e84ff28..c69453b 100644 (file)
--- a/src/coreclr/jit/inlinepolicy.cpp
+++ b/src/coreclr/jit/inlinepolicy.cpp
@@ -1338,7 +1338,7 @@ void ExtendedDefaultPolicy::NoteInt(InlineObservation obs, int value)
             unsigned maxCodeSize = static_cast<unsigned>(JitConfig.JitExtDefaultPolicyMaxIL());
 
             // TODO: Enable for PgoSource::Static as well if it's not the generic profile we bundle.
-            if (m_HasProfile && (m_RootCompiler->fgPgoSource == ICorJitInfo::PgoSource::Dynamic))
+            if (m_HasProfile && (m_RootCompiler->fgHaveTrustedProfileData()))
             {
                 maxCodeSize = static_cast<unsigned>(JitConfig.JitExtDefaultPolicyMaxILProf());
             }
@@ -1684,9 +1684,8 @@ double ExtendedDefaultPolicy::DetermineMultiplier()
         const double profileTrustCoef = (double)JitConfig.JitExtDefaultPolicyProfTrust() / 10.0;
         const double profileScale     = (double)JitConfig.JitExtDefaultPolicyProfScale() / 10.0;
 
-        if (m_RootCompiler->fgPgoSource == ICorJitInfo::PgoSource::Dynamic)
+        if (m_RootCompiler->fgHaveTrustedProfileData())
         {
-            // For now we only "trust" dynamic profiles.
             multiplier *= (1.0 - profileTrustCoef) + min(m_ProfileFrequency, 1.0) * profileScale;
         }
         else

diff --git a/src/coreclr/vm/pgo.cpp b/src/coreclr/vm/pgo.cpp
index b2dc0dc..189487c 100644 (file)
--- a/src/coreclr/vm/pgo.cpp
+++ b/src/coreclr/vm/pgo.cpp
@@ -718,24 +718,11 @@ HRESULT PgoManager::getPgoInstrumentationResults(MethodDesc* pMD, BYTE** pAlloca
     *pCountSchemaItems = 0;
     *pPgoSource = ICorJitInfo::PgoSource::Unknown;
 
-    PgoManager *mgr;
-    if (!pMD->IsDynamicMethod())
-    {
-        mgr = pMD->GetLoaderAllocator()->GetPgoManager();
-    }
-    else
-    {
-        mgr = pMD->AsDynamicMethodDesc()->GetResolver()->GetDynamicPgoManager();
-    }
-
     HRESULT hr = E_NOTIMPL;
-    if (mgr != NULL)
-    {
-        hr = mgr->getPgoInstrumentationResultsInstance(pMD, pAllocatedData, ppSchema, pCountSchemaItems, pInstrumentationData, pPgoSource);
-    }
 
-    // If not found in the data from the current run, look in the data from the text file
-    if (FAILED(hr) && s_textFormatPgoData.GetCount() > 0)
+    // If there is text format PGO data, prefer that over any dynamic or static data.
+    //
+    if (s_textFormatPgoData.GetCount() > 0)
     {
         COUNT_T methodhash = pMD->GetStableHash();
         int codehash;
@@ -796,11 +783,12 @@ HRESULT PgoManager::getPgoInstrumentationResults(MethodDesc* pMD, BYTE** pAlloca
                         *pAllocatedData = new BYTE[schemaArray.GetCount() * sizeof(ICorJitInfo::PgoInstrumentationSchema)];
                         memcpy(*pAllocatedData, schemaArray.OpenRawBuffer(), schemaArray.GetCount() * sizeof(ICorJitInfo::PgoInstrumentationSchema));
                         schemaArray.CloseRawBuffer();
-                        *ppSchema = (ICorJitInfo::PgoInstrumentationSchema*)pAllocatedData;
+                        *ppSchema = (ICorJitInfo::PgoInstrumentationSchema*)*pAllocatedData;
 
                         *pCountSchemaItems = schemaArray.GetCount();
                         *pInstrumentationData = found->GetData();
                         *pPgoSource = ICorJitInfo::PgoSource::Text;
+
                         hr = S_OK;
                     }
                     EX_CATCH
@@ -818,6 +806,26 @@ HRESULT PgoManager::getPgoInstrumentationResults(MethodDesc* pMD, BYTE** pAlloca
         }
     }
 
+    // If we didn't find any text format data, look for dynamic or static data.
+    //
+    if (FAILED(hr))
+    {
+        PgoManager *mgr;
+        if (!pMD->IsDynamicMethod())
+        {
+            mgr = pMD->GetLoaderAllocator()->GetPgoManager();
+        }
+        else
+        {
+            mgr = pMD->AsDynamicMethodDesc()->GetResolver()->GetDynamicPgoManager();
+        }
+
+        if (mgr != NULL)
+        {
+            hr = mgr->getPgoInstrumentationResultsInstance(pMD, pAllocatedData, ppSchema, pCountSchemaItems, pInstrumentationData, pPgoSource);
+        }
+    }
+
     return hr;
 }