Merge pull request #4625 from AndyAyersMS/FullInliner

Inliner: implement FullPolicy

src/jit/compiler.h

@@ -8600,8 +8600,10 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
     static fgWalkPreFn      gsMarkPtrsAndAssignGroups;  // Shadow param analysis tree-walk
     static fgWalkPreFn      gsReplaceShadowParams;      // Shadow param replacement tree-walk
 
-#define DEFAULT_MAX_INLINE_SIZE         100         // Method with >  DEFAULT_MAX_INLINE_SIZE IL bytes will never be inlined.
+#define DEFAULT_MAX_INLINE_SIZE         100         // Methods with >  DEFAULT_MAX_INLINE_SIZE IL bytes will never be inlined.
                                                     // This can be overwritten by setting complus_JITInlineSize env variable.
+
+#define DEFAULT_MAX_INLINE_DEPTH         20         // Methods at more than this level deep will not be inlined
 
 private:
 #ifdef FEATURE_JIT_METHOD_PERF

src/jit/flowgraph.cpp

@@ -21329,7 +21329,7 @@ void Compiler::fgRemoveContainedEmbeddedStatements(GenTreePtr tree, GenTreeStmt*
 
 //------------------------------------------------------------------------
 // fgCheckForInlineDepthAndRecursion: compute depth of the candidate, and
-// check for recursion and excessive depth
+// check for recursion.
 //
 // Return Value:
 //    The depth of the inline candidate. The root method is a depth 0, top-level
@@ -21350,13 +21350,11 @@ unsigned     Compiler::fgCheckInlineDepthAndRecursion(InlineInfo* inlineInfo)
 
     // There should be a context for all candidates.
     assert(inlineContext != nullptr);
-
-    const DWORD MAX_INLINING_RECURSION_DEPTH = 20;
-    DWORD depth = 0;
+    int depth = 0;
 
     for (; inlineContext != nullptr; inlineContext = inlineContext->GetParent())
     {
-        // Hard limit just to catch pathological cases
+
         depth++;
 
         if (inlineContext->GetCode() == candidateCode)
@@ -21367,9 +21365,8 @@ unsigned     Compiler::fgCheckInlineDepthAndRecursion(InlineInfo* inlineInfo)
             break;
         }
 
-        if (depth > MAX_INLINING_RECURSION_DEPTH)
+        if (depth > InlineStrategy::IMPLEMENTATION_MAX_INLINE_DEPTH)
         {
-            inlineResult->NoteFatal(InlineObservation::CALLSITE_IS_TOO_DEEP);
             break;
         }
     }

src/jit/inline.cpp

@@ -706,6 +706,7 @@ InlineStrategy::InlineStrategy(Compiler* compiler)
     , m_InlineAttemptCount(0)
     , m_InlineCount(0)
     , m_MaxInlineSize(DEFAULT_MAX_INLINE_SIZE)
+    , m_MaxInlineDepth(DEFAULT_MAX_INLINE_DEPTH)
     , m_InitialTimeBudget(0)
     , m_InitialTimeEstimate(0)
     , m_CurrentTimeBudget(0)
@@ -741,6 +742,18 @@ InlineStrategy::InlineStrategy(Compiler* compiler)
     assert(m_MaxInlineSize >= ALWAYS_INLINE_SIZE);
     assert(m_MaxInlineSize <= IMPLEMENTATION_MAX_INLINE_SIZE);
 
+    // Possibly modify the max inline depth
+    //
+    // Default value of JitInlineDepth is the same as our default.
+    // So normally this next line does not change the size.
+    m_MaxInlineDepth = JitConfig.JitInlineDepth();
+
+    // But don't overdo it
+    if (m_MaxInlineDepth > IMPLEMENTATION_MAX_INLINE_DEPTH)
+    {
+        m_MaxInlineDepth = IMPLEMENTATION_MAX_INLINE_DEPTH;
+    }
+
 #endif // DEBUG
 
 }
@@ -1165,7 +1178,7 @@ void InlineStrategy::Dump()
 
 // Static to track emission of the inline data header
 
-bool InlineStrategy::s_DumpDataHeader = false;
+bool InlineStrategy::s_HasDumpedDataHeader = false;
 
 //------------------------------------------------------------------------
 // DumpData: dump data about the last successful inline into this method
@@ -1209,7 +1222,7 @@ void InlineStrategy::DumpData()
         m_LastSuccessfulPolicy->NoteInt(InlineObservation::CALLEE_IL_CODE_SIZE, info.compMethodInfo->ILCodeSize);
     }
 
-    if (!s_DumpDataHeader)
+    if (!s_HasDumpedDataHeader)
     {
         if (limit == 0)
         {
@@ -1222,7 +1235,7 @@ void InlineStrategy::DumpData()
             fprintf(stderr, "\n");
         }
 
-        s_DumpDataHeader = true;
+        s_HasDumpedDataHeader = true;
     }
 
     // We'd really like the method identifier to be unique and
@@ -1259,10 +1272,10 @@ void InlineStrategy::DumpData()
 
 // Static to track emission of the xml data header
 
-bool InlineStrategy::s_DumpXmlHeader = false;
+bool InlineStrategy::s_HasDumpedXmlHeader = false;
 
 //------------------------------------------------------------------------
-// DumpXML: dump xml-formatted version of the inline tree.
+// DumpXml: dump xml-formatted version of the inline tree.
 //
 // Arguments
 //    file - file for data output
@@ -1276,12 +1289,12 @@ void InlineStrategy::DumpXml(FILE* file, unsigned indent)
     }
 
     // Dump header
-    if (!s_DumpXmlHeader)
+    if (!s_HasDumpedXmlHeader)
     {
         fprintf(file, "<?xml version=\"1.0\"?>\n");
         fprintf(file, "<InlineForest>\n");
         fprintf(file, "<Methods>\n");
-        s_DumpXmlHeader = true;
+        s_HasDumpedXmlHeader = true;
     }
 
     // Cache references to compiler substructures.
@@ -1344,17 +1357,23 @@ void InlineStrategy::DumpXml(FILE* file, unsigned indent)
     fprintf(file, "%*s</Method>\n", indent, "");
 }
 
+//------------------------------------------------------------------------
+// FinalizeXml: finalize the xml-formatted version of the inline tree.
+//
+// Arguments
+//    file - file for data output
+
 void InlineStrategy::FinalizeXml(FILE* file)
 {
     // If we dumped the header, dump a footer
-    if (s_DumpXmlHeader)
+    if (s_HasDumpedXmlHeader)
     {
         fprintf(file, "</Methods>\n");
         fprintf(file, "</InlineForest>\n");
         fflush(file);
 
         // Workaroud compShutdown getting called twice.
-        s_DumpXmlHeader = false;
+        s_HasDumpedXmlHeader = false;
     }
 }
 

src/jit/inline.h

@@ -670,6 +670,12 @@ class InlineStrategy
         return m_MaxInlineSize;
     }
 
+    // Get depth of maximum allowable inline
+    unsigned GetMaxInlineDepth()
+    {
+        return m_MaxInlineDepth;
+    }
+
     // Number of successful inlines into the root.
     unsigned GetInlineCount()
     {
@@ -701,7 +707,8 @@ class InlineStrategy
     enum
     {
         ALWAYS_INLINE_SIZE = 16,
-        IMPLEMENTATION_MAX_INLINE_SIZE= _UI16_MAX
+        IMPLEMENTATION_MAX_INLINE_SIZE = _UI16_MAX,
+        IMPLEMENTATION_MAX_INLINE_DEPTH = 1000
     };
 
 private:
@@ -731,8 +738,8 @@ class InlineStrategy
     int EstimateSize(InlineContext* context);
 
 #if defined(DEBUG) || defined(INLINE_DATA)
-    static bool    s_DumpDataHeader;
-    static bool    s_DumpXmlHeader;
+    static bool    s_HasDumpedDataHeader;
+    static bool    s_HasDumpedXmlHeader;
 #endif // defined(DEBUG) || defined(INLINE_DATA)
 
     Compiler*      m_Compiler;
@@ -742,6 +749,7 @@ class InlineStrategy
     unsigned       m_InlineAttemptCount;
     unsigned       m_InlineCount;
     unsigned       m_MaxInlineSize;
+    unsigned       m_MaxInlineDepth;
     int            m_InitialTimeBudget;
     int            m_InitialTimeEstimate;
     int            m_CurrentTimeBudget;

src/jit/inlinepolicy.cpp

@@ -43,6 +43,14 @@ InlinePolicy* InlinePolicy::GetPolicy(Compiler* compiler, bool isPrejitRoot)
 
 #if defined(DEBUG) || defined(INLINE_DATA)
 
+    // Optionally install the FullPolicy.
+    bool useFullPolicy = JitConfig.JitInlinePolicyFull() != 0;
+
+    if (useFullPolicy)
+    {
+        return new (compiler, CMK_Inlining) FullPolicy(compiler, isPrejitRoot);
+    }
+
     // Optionally install the ModelPolicy.
     bool useModelPolicy = JitConfig.JitInlinePolicyModel() != 0;
 
@@ -439,6 +447,18 @@ void LegacyPolicy::NoteInt(InlineObservation obs, int value)
             break;
         }
 
+    case InlineObservation::CALLSITE_DEPTH:
+        {
+            unsigned depth = static_cast<unsigned>(value);
+
+            if (depth > m_RootCompiler->m_inlineStrategy->GetMaxInlineDepth())
+            {
+                SetFailure(InlineObservation::CALLSITE_IS_TOO_DEEP);
+            }
+
+            break;
+        }
+
     case InlineObservation::CALLEE_OPCODE_NORMED:
     case InlineObservation::CALLEE_OPCODE:
         {
@@ -1873,4 +1893,59 @@ void ModelPolicy::DetermineProfitability(CORINFO_METHOD_INFO* methodInfo)
     }
 }
 
+//------------------------------------------------------------------------/
+// FullPolicy: construct a new FullPolicy
+//
+// Arguments:
+//    compiler -- compiler instance doing the inlining (root compiler)
+//    isPrejitRoot -- true if this compiler is prejitting the root method
+
+FullPolicy::FullPolicy(Compiler* compiler, bool isPrejitRoot)
+    : DiscretionaryPolicy(compiler, isPrejitRoot)
+{
+    // Empty
+}
+
+//------------------------------------------------------------------------
+// DetermineProfitability: determine if this inline is profitable
+//
+// Arguments:
+//    methodInfo -- method info for the callee
+
+void FullPolicy::DetermineProfitability(CORINFO_METHOD_INFO* methodInfo)
+{
+    // Check depth
+
+    unsigned depthLimit = m_RootCompiler->m_inlineStrategy->GetMaxInlineDepth();
+
+    if (m_Depth > depthLimit) 
+    {
+        SetFailure(InlineObservation::CALLSITE_IS_TOO_DEEP);
+        return;
+    }
+
+    // Check size
+
+    unsigned sizeLimit = m_RootCompiler->m_inlineStrategy->GetMaxInlineILSize();
+
+    if (m_CodeSize > sizeLimit)
+    {
+        SetFailure(InlineObservation::CALLEE_TOO_MUCH_IL);
+        return;
+    }
+
+    // Otherwise, we're good to go
+
+    if (m_IsPrejitRoot)
+    {
+        SetCandidate(InlineObservation::CALLEE_IS_PROFITABLE_INLINE);
+    }
+    else
+    {
+        SetCandidate(InlineObservation::CALLSITE_IS_PROFITABLE_INLINE);
+    }
+
+    return;
+}
+
 #endif // defined(DEBUG) || defined(INLINE_DATA)

src/jit/inlinepolicy.h

@@ -11,9 +11,14 @@
 //
 // LegalPolicy         - partial class providing common legality checks
 // LegacyPolicy        - policy that provides legacy inline behavior
+//
+// These experimental policies are available only in
+// DEBUG or release+INLINE_DATA builds of the jit.
+//
 // RandomPolicy        - randomized inlining
 // DiscretionaryPolicy - legacy variant with uniform size policy
 // ModelPolicy         - policy based on statistical modelling
+// FullPolicy          - inlines everything up to size and depth limits
 
 #ifndef _INLINE_POLICY_H_
 #define _INLINE_POLICY_H_
@@ -292,6 +297,26 @@ class ModelPolicy : public DiscretionaryPolicy
     const char* GetName() const override { return "ModelPolicy"; }
 };
 
+// FullPolicy is an experimental policy that will always inline if
+// possible, subject to externally settable depth and size limits.
+//
+// It's useful for unconvering the full set of possible inlines for
+// methods.
+
+class FullPolicy : public DiscretionaryPolicy
+{
+public:
+
+    // Construct a ModelPolicy
+    FullPolicy(Compiler* compiler, bool isPrejitRoot);
+
+    // Policy determinations
+    void DetermineProfitability(CORINFO_METHOD_INFO* methodInfo) override;
+
+    // Miscellaneous
+    const char* GetName() const override { return "FullPolicy"; }
+};
+
 #endif // defined(DEBUG) || defined(INLINE_DATA)
 
 #endif // _INLINE_POLICY_H_

src/jit/jitconfigvalues.h

@@ -57,6 +57,7 @@ CONFIG_INTEGER(JitHashHalt, W("JitHashHalt"), -1) // Same as JitHalt, but for a
 CONFIG_INTEGER(JitInlineAdditionalMultiplier, W("JitInlineAdditionalMultiplier"), 0)
 CONFIG_INTEGER(JitInlinePrintStats, W("JitInlinePrintStats"), 0)
 CONFIG_INTEGER(JitInlineSize, W("JITInlineSize"), DEFAULT_MAX_INLINE_SIZE)
+CONFIG_INTEGER(JitInlineDepth, W("JITInlineDepth"), DEFAULT_MAX_INLINE_DEPTH)
 CONFIG_INTEGER(JitLargeBranches, W("JitLargeBranches"), 0) // Force using the largest conditional branch format
 CONFIG_INTEGER(JitMaxTempAssert, W("JITMaxTempAssert"), 1)
 CONFIG_INTEGER(JitMaxUncheckedOffset, W("JitMaxUncheckedOffset"), 8)
@@ -195,6 +196,7 @@ CONFIG_INTEGER(JitInlineDumpXml, W("JitInlineDumpXml"), 0)
 CONFIG_INTEGER(JitInlineLimit, W("JitInlineLimit"), -1)
 CONFIG_INTEGER(JitInlinePolicyDiscretionary, W("JitInlinePolicyDiscretionary"), 0)
 CONFIG_INTEGER(JitInlinePolicyModel, W("JitInlinePolicyModel"), 0)
+CONFIG_INTEGER(JitInlinePolicyFull, W("JitInlinePolicyFull"), 0)
 #endif // defined(DEBUG) || defined(INLINE_DATA)
 
 #undef CONFIG_INTEGER