Inliner: updates to RandomPolicy (#8128)

Fix issues with RandomPolicy setup in release builds with -DINLINE_DATA.

Reparent RandomPolicy on top of DiscretionaryPolicy to enable inline
data dumps from random inline runs. Remove some now-unneeded overrides.

Add a full dump mode to JitInlineDumpData that reports the inliner-visible
data for all inlines, as opposed to inliner-visible and post-inline
data for just the most recent inline. Small mods to the dumper code to
adjust comma placement for this new mode.

Have the RandomPolicy make the full set of profitability observations
for each accepted inline so they can be dumped.

1 files changed, 23 insertions, 79 deletions

diff --git a/src/jit/inlinepolicy.cpp b/src/jit/inlinepolicy.cpp
index a909845177..d108b987e0 100644
--- a/src/jit/inlinepolicy.cpp
+++ b/src/jit/inlinepolicy.cpp
@@ -27,21 +27,22 @@
 InlinePolicy* InlinePolicy::GetPolicy(Compiler* compiler, bool isPrejitRoot)
 {
 
-#ifdef DEBUG
+#if defined(DEBUG) || defined(INLINE_DATA)
 
-    // Optionally install the RandomPolicy.
+#if defined(DEBUG)
     const bool useRandomPolicyForStress = compiler->compRandomInlineStress();
-    const bool useRandomPolicy          = (JitConfig.JitInlinePolicyRandom() != 0);
+#else
+    const bool useRandomPolicyForStress = false;
+#endif // defined(DEBUG)
 
+    const bool useRandomPolicy = (JitConfig.JitInlinePolicyRandom() != 0);
+
+    // Optionally install the RandomPolicy.
     if (useRandomPolicyForStress || useRandomPolicy)
     {
         return new (compiler, CMK_Inlining) RandomPolicy(compiler, isPrejitRoot);
     }
 
-#endif // DEBUG
-
-#if defined(DEBUG) || defined(INLINE_DATA)
-
     // Optionally install the ReplayPolicy.
     bool useReplayPolicy = JitConfig.JitInlinePolicyReplay() != 0;
 
@@ -982,78 +983,12 @@ bool EnhancedLegacyPolicy::PropagateNeverToRuntime() const
 //    compiler -- compiler instance doing the inlining (root compiler)
 //    isPrejitRoot -- true if this compiler is prejitting the root method
 
-RandomPolicy::RandomPolicy(Compiler* compiler, bool isPrejitRoot)
-    : LegalPolicy(isPrejitRoot)
-    , m_RootCompiler(compiler)
-    , m_Random(nullptr)
-    , m_CodeSize(0)
-    , m_IsForceInline(false)
-    , m_IsForceInlineKnown(false)
+RandomPolicy::RandomPolicy(Compiler* compiler, bool isPrejitRoot) : DiscretionaryPolicy(compiler, isPrejitRoot)
 {
     m_Random = compiler->m_inlineStrategy->GetRandom();
 }
 
 //------------------------------------------------------------------------
-// NoteSuccess: handle finishing all the inlining checks successfully
-
-void RandomPolicy::NoteSuccess()
-{
-    assert(InlDecisionIsCandidate(m_Decision));
-    m_Decision = InlineDecision::SUCCESS;
-}
-
-//------------------------------------------------------------------------
-// NoteBool: handle a boolean observation with non-fatal impact
-//
-// Arguments:
-//    obs      - the current obsevation
-//    value    - the value of the observation
-void RandomPolicy::NoteBool(InlineObservation obs, bool value)
-{
-    // Check the impact
-    InlineImpact impact = InlGetImpact(obs);
-
-    // As a safeguard, all fatal impact must be
-    // reported via NoteFatal.
-    assert(impact != InlineImpact::FATAL);
-
-    // Handle most information here
-    bool isInformation = (impact == InlineImpact::INFORMATION);
-    bool propagate     = !isInformation;
-
-    if (isInformation)
-    {
-        switch (obs)
-        {
-            case InlineObservation::CALLEE_IS_FORCE_INLINE:
-                // The RandomPolicy still honors force inlines.
-                //
-                // We may make the force-inline observation more than
-                // once.  All observations should agree.
-                assert(!m_IsForceInlineKnown || (m_IsForceInline == value));
-                m_IsForceInline      = value;
-                m_IsForceInlineKnown = true;
-                break;
-
-            case InlineObservation::CALLEE_HAS_SWITCH:
-            case InlineObservation::CALLEE_UNSUPPORTED_OPCODE:
-                // Pass these on, they should cause inlining to fail.
-                propagate = true;
-                break;
-
-            default:
-                // Ignore the remainder for now
-                break;
-        }
-    }
-
-    if (propagate)
-    {
-        NoteInternal(obs);
-    }
-}
-
-//------------------------------------------------------------------------
 // NoteInt: handle an observed integer value
 //
 // Arguments:
@@ -1064,7 +999,6 @@ void RandomPolicy::NoteInt(InlineObservation obs, int value)
 {
     switch (obs)
     {
-
         case InlineObservation::CALLEE_IL_CODE_SIZE:
         {
             assert(m_IsForceInlineKnown);
@@ -1086,7 +1020,8 @@ void RandomPolicy::NoteInt(InlineObservation obs, int value)
         }
 
         default:
-            // Ignore all other information
+            // Defer to superclass for all other information
+            DiscretionaryPolicy::NoteInt(obs, value);
             break;
     }
 }
@@ -1119,6 +1054,16 @@ void RandomPolicy::DetermineProfitability(CORINFO_METHOD_INFO* methodInfo)
         }
     }
 
+    // If we're also dumping inline data, make additional observations
+    // based on the method info, and estimate code size and perf
+    // impact, so that the reports have the necessary data.
+    if (JitConfig.JitInlineDumpData() != 0)
+    {
+        MethodInfoObservations(methodInfo);
+        EstimateCodeSize();
+        EstimatePerformanceImpact();
+    }
+
     // Use a probability curve that roughly matches the observed
     // behavior of the LegacyPolicy. That way we're inlining
     // differently but not creating enormous methods.
@@ -1337,7 +1282,6 @@ void DiscretionaryPolicy::NoteInt(InlineObservation obs, int value)
 {
     switch (obs)
     {
-
         case InlineObservation::CALLEE_IL_CODE_SIZE:
             // Override how code size is handled
             {
@@ -1911,7 +1855,7 @@ int DiscretionaryPolicy::CodeSizeEstimate()
 
 void DiscretionaryPolicy::DumpSchema(FILE* file) const
 {
-    fprintf(file, ",ILSize");
+    fprintf(file, "ILSize");
     fprintf(file, ",CallsiteFrequency");
     fprintf(file, ",InstructionCount");
     fprintf(file, ",LoadStoreCount");
@@ -1993,7 +1937,7 @@ void DiscretionaryPolicy::DumpSchema(FILE* file) const
 
 void DiscretionaryPolicy::DumpData(FILE* file) const
 {
-    fprintf(file, ",%u", m_CodeSize);
+    fprintf(file, "%u", m_CodeSize);
     fprintf(file, ",%u", m_CallsiteFrequency);
     fprintf(file, ",%u", m_InstructionCount);
     fprintf(file, ",%u", m_LoadStoreCount);