JIT: make suitably optimistic prejit inline assessments (#14850)

When prejitting, the jit assesses whether each root method is a potential
inline candidate for any possible caller. Methods deemed un-inlinable in any
caller are marked in the runtime as "noinline" to save the jit some work
later on when it sees calls to these methods.

This assessment was too conservative and led to prejit-ordering dependences
for inlines. It also meant that prejitting was missing some inlines that
would have happened had we not done the prejit root assessment.

This change removes some of the prejit observation blockers. These mostly
will enable more prejit methods to become candidates. We also now track when
a method argument reaches a test.

When we are assessing profitability for a prejit root, assume the call site
best possible case.

Also, update the inline XML to capture the prejit assessments.

This increases the number of inlines considered and performed when prejitting
and will also lead to slightly more inlining when jitting. However we do not
expect a large througput impact when jitting -- the main impact of this change
is that inlining when prejitting is now just as aggressive as inlining when
jitting, and the decisions no longer depend on the order in which methods are
prejitted.

Closes #14441.
Closes #3482.

8 files changed, 75 insertions, 64 deletions

diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp
index 35cfcf6a73..385fe4c593 100644
--- a/src/jit/compiler.cpp
+++ b/src/jit/compiler.cpp
@@ -6064,6 +6064,8 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE            classPtr,
             prejitResult.DetermineProfitability(methodInfo);
         }
 
+        m_inlineStrategy->NotePrejitDecision(prejitResult);
+
         // Handle the results of the inline analysis.
         if (prejitResult.IsFailure())
         {
diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp
index 60d4d06354..bb222c42dd 100644
--- a/src/jit/flowgraph.cpp
+++ b/src/jit/flowgraph.cpp
@@ -5107,6 +5107,8 @@ void Compiler::fgObserveInlineConstants(OPCODE opcode, const FgStack& stack, boo
     {
         if (FgStack::IsArgument(slot0))
         {
+            compInlineResult->Note(InlineObservation::CALLEE_ARG_FEEDS_TEST);
+
             unsigned varNum = FgStack::SlotTypeToArgNum(slot0);
             if (impInlineInfo->inlArgInfo[varNum].argNode->OperIsConst())
             {
@@ -5116,6 +5118,8 @@ void Compiler::fgObserveInlineConstants(OPCODE opcode, const FgStack& stack, boo
 
         if (FgStack::IsArgument(slot1))
         {
+            compInlineResult->Note(InlineObservation::CALLEE_ARG_FEEDS_TEST);
+
             unsigned varNum = FgStack::SlotTypeToArgNum(slot1);
             if (impInlineInfo->inlArgInfo[varNum].argNode->OperIsConst())
             {
diff --git a/src/jit/inline.cpp b/src/jit/inline.cpp
index 05fcf1c6b9..679ff41240 100644
--- a/src/jit/inline.cpp
+++ b/src/jit/inline.cpp
@@ -474,6 +474,12 @@ void InlineContext::DumpXml(FILE* file, unsigned indent)
         m_Sibling->DumpXml(file, indent);
     }
 
+    // Optionally suppress failing inline records
+    if ((JitConfig.JitInlineDumpXml() == 3) && !m_Success)
+    {
+        return;
+    }
+
     const bool  isRoot     = m_Parent == nullptr;
     const bool  hasChild   = m_Child != nullptr;
     const char* inlineType = m_Success ? "Inline" : "FailedInline";
@@ -739,6 +745,8 @@ InlineStrategy::InlineStrategy(Compiler* compiler)
     : m_Compiler(compiler)
     , m_RootContext(nullptr)
     , m_LastSuccessfulPolicy(nullptr)
+    , m_LastContext(nullptr)
+    , m_PrejitRootDecision(InlineDecision::UNDECIDED)
     , m_CallCount(0)
     , m_CandidateCount(0)
     , m_AlwaysCandidateCount(0)
@@ -1493,7 +1501,7 @@ void InlineStrategy::DumpXml(FILE* file, unsigned indent)
     // If we're dumping "minimal" Xml, and we didn't do
     // any inlines into this method, then there's nothing
     // to emit here.
-    if ((m_InlineCount == 0) && (JitConfig.JitInlineDumpXml() == 2))
+    if ((m_InlineCount == 0) && (JitConfig.JitInlineDumpXml() >= 2))
     {
         return;
     }
@@ -1566,6 +1574,15 @@ void InlineStrategy::DumpXml(FILE* file, unsigned indent)
     fprintf(file, "%*s<SizeEstimate>%u</SizeEstimate>\n", indent + 2, "", m_CurrentSizeEstimate / 10);
     fprintf(file, "%*s<TimeEstimate>%u</TimeEstimate>\n", indent + 2, "", m_CurrentTimeEstimate);
 
+    // For prejit roots also propagate out the assessment of the root method
+    if (isPrejitRoot)
+    {
+        fprintf(file, "%*s<PrejitDecision>%s</PrejitDecision>\n", indent + 2, "",
+                InlGetDecisionString(m_PrejitRootDecision));
+        fprintf(file, "%*s<PrejitObservation>%s</PrejitObservation>\n", indent + 2, "",
+                InlGetObservationString(m_PrejitRootObservation));
+    }
+
     // Root context will be null if we're not optimizing the method.
     //
     // Note there are cases of this in mscorlib even in release builds,
diff --git a/src/jit/inline.def b/src/jit/inline.def
index 0a13950b48..4dd67402fb 100644
--- a/src/jit/inline.def
+++ b/src/jit/inline.def
@@ -72,6 +72,7 @@ INLINE_OBSERVATION(TOO_MUCH_IL,               bool,   "too many il bytes",
 // ------ Callee Information ------- 
 
 INLINE_OBSERVATION(ARG_FEEDS_CONSTANT_TEST,   bool,   "argument feeds constant test",  INFORMATION, CALLEE)
+INLINE_OBSERVATION(ARG_FEEDS_TEST,            bool,   "argument feeds test",           INFORMATION, CALLEE)
 INLINE_OBSERVATION(ARG_FEEDS_RANGE_CHECK,     bool,   "argument feeds range check",    INFORMATION, CALLEE)
 INLINE_OBSERVATION(BEGIN_OPCODE_SCAN,         bool,   "prepare to look at opcodes",    INFORMATION, CALLEE)
 INLINE_OBSERVATION(BELOW_ALWAYS_INLINE_SIZE,  bool,   "below ALWAYS_INLINE size",      INFORMATION, CALLEE)
diff --git a/src/jit/inline.h b/src/jit/inline.h
index f06b4f7a6f..757efec6a5 100644
--- a/src/jit/inline.h
+++ b/src/jit/inline.h
@@ -782,6 +782,13 @@ public:
         m_ImportCount++;
     }
 
+    // Inform strategy about the inline decision for a prejit root
+    void NotePrejitDecision(const InlineResult& r)
+    {
+        m_PrejitRootDecision    = r.GetPolicy()->GetDecision();
+        m_PrejitRootObservation = r.GetPolicy()->GetObservation();
+    }
+
     // Dump csv header for inline stats to indicated file.
     static void DumpCsvHeader(FILE* f);
 
@@ -866,27 +873,29 @@ private:
     static CritSecObject s_XmlWriterLock;
 #endif // defined(DEBUG) || defined(INLINE_DATA)
 
-    Compiler*      m_Compiler;
-    InlineContext* m_RootContext;
-    InlinePolicy*  m_LastSuccessfulPolicy;
-    InlineContext* m_LastContext;
-    unsigned       m_CallCount;
-    unsigned       m_CandidateCount;
-    unsigned       m_AlwaysCandidateCount;
-    unsigned       m_ForceCandidateCount;
-    unsigned       m_DiscretionaryCandidateCount;
-    unsigned       m_UnprofitableCandidateCount;
-    unsigned       m_ImportCount;
-    unsigned       m_InlineCount;
-    unsigned       m_MaxInlineSize;
-    unsigned       m_MaxInlineDepth;
-    int            m_InitialTimeBudget;
-    int            m_InitialTimeEstimate;
-    int            m_CurrentTimeBudget;
-    int            m_CurrentTimeEstimate;
-    int            m_InitialSizeEstimate;
-    int            m_CurrentSizeEstimate;
-    bool           m_HasForceViaDiscretionary;
+    Compiler*         m_Compiler;
+    InlineContext*    m_RootContext;
+    InlinePolicy*     m_LastSuccessfulPolicy;
+    InlineContext*    m_LastContext;
+    InlineDecision    m_PrejitRootDecision;
+    InlineObservation m_PrejitRootObservation;
+    unsigned          m_CallCount;
+    unsigned          m_CandidateCount;
+    unsigned          m_AlwaysCandidateCount;
+    unsigned          m_ForceCandidateCount;
+    unsigned          m_DiscretionaryCandidateCount;
+    unsigned          m_UnprofitableCandidateCount;
+    unsigned          m_ImportCount;
+    unsigned          m_InlineCount;
+    unsigned          m_MaxInlineSize;
+    unsigned          m_MaxInlineDepth;
+    int               m_InitialTimeBudget;
+    int               m_InitialTimeEstimate;
+    int               m_CurrentTimeBudget;
+    int               m_CurrentTimeEstimate;
+    int               m_InitialSizeEstimate;
+    int               m_CurrentSizeEstimate;
+    bool              m_HasForceViaDiscretionary;
 
 #if defined(DEBUG) || defined(INLINE_DATA)
     long       m_MethodXmlFilePosition;
diff --git a/src/jit/inlinepolicy.cpp b/src/jit/inlinepolicy.cpp
index 5847bbcb54..eaf739612c 100644
--- a/src/jit/inlinepolicy.cpp
+++ b/src/jit/inlinepolicy.cpp
@@ -267,37 +267,24 @@ void DefaultPolicy::NoteBool(InlineObservation obs, bool value)
                 break;
 
             case InlineObservation::CALLEE_LOOKS_LIKE_WRAPPER:
-                // DefaultPolicy ignores this for prejit roots.
-                if (!m_IsPrejitRoot)
-                {
-                    m_LooksLikeWrapperMethod = value;
-                }
+                m_LooksLikeWrapperMethod = value;
+                break;
+
+            case InlineObservation::CALLEE_ARG_FEEDS_TEST:
+                m_ArgFeedsTest++;
                 break;
 
             case InlineObservation::CALLEE_ARG_FEEDS_CONSTANT_TEST:
-                // DefaultPolicy ignores this for prejit roots.
-                if (!m_IsPrejitRoot)
-                {
-                    m_ArgFeedsConstantTest++;
-                }
+                m_ArgFeedsConstantTest++;
                 break;
 
             case InlineObservation::CALLEE_ARG_FEEDS_RANGE_CHECK:
-                // DefaultPolicy ignores this for prejit roots.
-                if (!m_IsPrejitRoot)
-                {
-                    m_ArgFeedsRangeCheck++;
-                }
+                m_ArgFeedsRangeCheck++;
                 break;
 
             case InlineObservation::CALLEE_HAS_SWITCH:
             case InlineObservation::CALLEE_UNSUPPORTED_OPCODE:
-                // DefaultPolicy ignores these for prejit roots.
-                if (!m_IsPrejitRoot)
-                {
-                    // Pass these on, they should cause inlining to fail.
-                    propagate = true;
-                }
+                propagate = true;
                 break;
 
             case InlineObservation::CALLSITE_CONSTANT_ARG_FEEDS_TEST:
@@ -649,6 +636,13 @@ double DefaultPolicy::DetermineMultiplier()
         multiplier += 3.0;
         JITDUMP("\nInline candidate has const arg that feeds a conditional.  Multiplier increased to %g.", multiplier);
     }
+    // For prejit roots we do not see the call sites. To be suitably optimisitic
+    // assume that call sites may pass constants.
+    else if (m_IsPrejitRoot && ((m_ArgFeedsConstantTest > 0) || (m_ArgFeedsTest > 0)))
+    {
+        multiplier += 3.0;
+        JITDUMP("\nPrejit root candidate has arg that feeds a conditional.  Multiplier increased to %g.", multiplier);
+    }
 
     switch (m_CallsiteFrequency)
     {
@@ -1158,25 +1152,6 @@ void DiscretionaryPolicy::NoteBool(InlineObservation obs, bool value)
 {
     switch (obs)
     {
-        case InlineObservation::CALLEE_LOOKS_LIKE_WRAPPER:
-            m_LooksLikeWrapperMethod = value;
-            break;
-
-        case InlineObservation::CALLEE_ARG_FEEDS_CONSTANT_TEST:
-            assert(value);
-            m_ArgFeedsConstantTest++;
-            break;
-
-        case InlineObservation::CALLEE_ARG_FEEDS_RANGE_CHECK:
-            assert(value);
-            m_ArgFeedsRangeCheck++;
-            break;
-
-        case InlineObservation::CALLSITE_CONSTANT_ARG_FEEDS_TEST:
-            assert(value);
-            m_ConstantArgFeedsConstantTest++;
-            break;
-
         case InlineObservation::CALLEE_IS_CLASS_CTOR:
             m_IsClassCtor = value;
             break;
diff --git a/src/jit/inlinepolicy.h b/src/jit/inlinepolicy.h
index 0ff0b33279..b1094bbaaa 100644
--- a/src/jit/inlinepolicy.h
+++ b/src/jit/inlinepolicy.h
@@ -84,6 +84,7 @@ public:
         , m_CallsiteFrequency(InlineCallsiteFrequency::UNUSED)
         , m_InstructionCount(0)
         , m_LoadStoreCount(0)
+        , m_ArgFeedsTest(0)
         , m_ArgFeedsConstantTest(0)
         , m_ArgFeedsRangeCheck(0)
         , m_ConstantArgFeedsConstantTest(0)
@@ -148,6 +149,7 @@ protected:
     InlineCallsiteFrequency m_CallsiteFrequency;
     unsigned                m_InstructionCount;
     unsigned                m_LoadStoreCount;
+    unsigned                m_ArgFeedsTest;
     unsigned                m_ArgFeedsConstantTest;
     unsigned                m_ArgFeedsRangeCheck;
     unsigned                m_ConstantArgFeedsConstantTest;
diff --git a/src/jit/jitconfigvalues.h b/src/jit/jitconfigvalues.h
index bb764f4e50..e9b9f0d275 100644
--- a/src/jit/jitconfigvalues.h
+++ b/src/jit/jitconfigvalues.h
@@ -310,8 +310,9 @@ CONFIG_STRING(JitMeasureNowayAssertFile,
 
 #if defined(DEBUG) || defined(INLINE_DATA)
 CONFIG_INTEGER(JitInlineDumpData, W("JitInlineDumpData"), 0)
-CONFIG_INTEGER(JitInlineDumpXml, W("JitInlineDumpXml"), 0) // 1 = full xml (all methods), 2 = minimal xml (only method
-                                                           // with inlines)
+CONFIG_INTEGER(JitInlineDumpXml, W("JitInlineDumpXml"), 0) // 1 = full xml (+ failures in DEBUG)
+                                                           // 2 = only methods with inlines (+ failures in DEBUG)
+                                                           // 3 = only methods with inlines, no failures
 CONFIG_INTEGER(JitInlineLimit, W("JitInlineLimit"), -1)
 CONFIG_INTEGER(JitInlinePolicyDiscretionary, W("JitInlinePolicyDiscretionary"), 0)
 CONFIG_INTEGER(JitInlinePolicyFull, W("JitInlinePolicyFull"), 0)