Inline refactoring: split up depth and recursion checks
authorAndy Ayers <andya@microsoft.com>
Sat, 20 Feb 2016 19:21:08 +0000 (11:21 -0800)
committerAndy Ayers <andya@microsoft.com>
Sun, 21 Feb 2016 19:44:14 +0000 (11:44 -0800)
Separate out the recursive inline and inline too deep obervations.
Have the checker update the inline result directly rather than
deferring to the caller.

Add a bit more commentary on how the various inline classes
are used during compilation.

Commit migrated from https://github.com/dotnet/coreclr/commit/336832d2119d3877f633891b367315ee935afdd7

src/coreclr/src/jit/compiler.h
src/coreclr/src/jit/flowgraph.cpp
src/coreclr/src/jit/inline.def
src/coreclr/src/jit/inline.h

index 54031ae..f099811 100644 (file)
@@ -4638,13 +4638,10 @@ private:
 
     unsigned            fgBigOffsetMorphingTemps[TYP_COUNT];
 
-    static bool         fgIsUnboundedInlineRecursion(InlineContext* context,
-                                                     BYTE*          ilCode,
-                                                     DWORD*         depth);
-
-    void                fgInvokeInlineeCompiler(GenTreeCall*   call, InlineResult* result);
-    void                fgInsertInlineeBlocks (InlineInfo * pInlineInfo);
-    GenTreePtr          fgInlinePrependStatements(InlineInfo * inlineInfo);
+    unsigned            fgCheckInlineDepthAndRecursion(InlineInfo* inlineInfo);
+    void                fgInvokeInlineeCompiler(GenTreeCall* call, InlineResult* result);
+    void                fgInsertInlineeBlocks (InlineInfo* pInlineInfo);
+    GenTreePtr          fgInlinePrependStatements(InlineInfo* inlineInfo);
 
 #if defined(_TARGET_ARM_) || defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)
     GenTreePtr          fgGetStructAsStructPtr(GenTreePtr tree);
index a8282ca..344d716 100644 (file)
@@ -21311,33 +21311,55 @@ void Compiler::fgRemoveContainedEmbeddedStatements(GenTreePtr tree, GenTreeStmt*
     }
 }
 
-/*****************************************************************************
- * Check if we have a recursion loop or if the recursion is too deep while doing the inlining.
- * Return true if a recursion loop is found or if the inlining recursion is too deep.
- * Also return the depth.
- */
+//------------------------------------------------------------------------
+// fgCheckForInlineDepthAndRecursion: compute depth of the candidate, and
+// check for recursion and excessive depth
+//
+// Return Value:
+//    The depth of the inline candidate. The root method is a depth 0, top-level
+//    candidates at depth 1, etc.
+//
+// Notes:
+//    We generally disallow recursive inlines by policy. However, they are
+//    supported by the underlying machinery.
+//
+//    Likewise the depth limit is a policy consideration, and serves mostly
+//    as a safeguard to prevent runaway inlining of small methods.
 
-bool          Compiler::fgIsUnboundedInlineRecursion(InlineContext*          inlineContext,
-                                                     BYTE*                   ilCode,
-                                                     DWORD*                  finalDepth)
+unsigned     Compiler::fgCheckInlineDepthAndRecursion(InlineInfo* inlineInfo)
 {
+    BYTE*          candidateCode = inlineInfo->inlineCandidateInfo->methInfo.ILCode;
+    InlineContext* inlineContext = inlineInfo->iciStmt->gtStmt.gtInlineContext;
+    InlineResult*  inlineResult  = inlineInfo->inlineResult;
+
+    // There should be a context for all candidates.
+    assert(inlineContext != nullptr);
+
     const DWORD MAX_INLINING_RECURSION_DEPTH = 20;
     DWORD depth = 0;
-    bool result = false;
 
     for (; inlineContext != nullptr; inlineContext = inlineContext->getParent())
     {
         // Hard limit just to catch pathological cases
         depth++;
-        if  ((inlineContext->getCode() == ilCode) || (depth > MAX_INLINING_RECURSION_DEPTH))
+
+        if (inlineContext->getCode() == candidateCode)
         {
-           result = true;
-           break;
+            // This inline candidate has the same IL code buffer as an already
+            // inlined method does.
+            inlineResult->noteFatal(InlineObservation::CALLSITE_IS_RECURSIVE);
+            break;
+        }
+
+        if (depth > MAX_INLINING_RECURSION_DEPTH)
+        {
+            inlineResult->noteFatal(InlineObservation::CALLSITE_IS_TOO_DEEP);
+            break;
         }
     }
 
-    *finalDepth = depth;
-    return result;
+    inlineResult->noteInt(InlineObservation::CALLSITE_DEPTH, depth);
+    return depth;
 }
 
 /*****************************************************************************
@@ -21757,11 +21779,9 @@ void       Compiler::fgInvokeInlineeCompiler(GenTreeCall*  call,
     // Store the link to inlineCandidateInfo into inlineInfo
     inlineInfo.inlineCandidateInfo = inlineCandidateInfo;
 
-    DWORD          inlineDepth = 0;
-    BYTE*          candidateIL = inlineCandidateInfo->methInfo.ILCode;
-    InlineContext* inlineContext = inlineInfo.iciStmt->gtStmt.gtInlineContext;
+    unsigned inlineDepth = fgCheckInlineDepthAndRecursion(&inlineInfo);
 
-    if (fgIsUnboundedInlineRecursion(inlineContext, candidateIL, &inlineDepth))
+    if (inlineResult->isFailure())
     {
 #ifdef DEBUG
         if (verbose)
@@ -21769,7 +21789,6 @@ void       Compiler::fgInvokeInlineeCompiler(GenTreeCall*  call,
             printf("Recursive or deep inline recursion detected. Will not expand this INLINECANDIDATE \n");
         }
 #endif // DEBUG
-        inlineResult->noteFatal(InlineObservation::CALLSITE_IS_RECURSIVE_OR_DEEP);
         return;
     }
 
index 64cb5a1..fa2ab24 100644 (file)
@@ -113,7 +113,8 @@ INLINE_OBSERVATION(IMPLICIT_REC_TAIL_CALL,    bool,   "implicit recursive tail c
 INLINE_OBSERVATION(IS_CALL_TO_HELPER,         bool,   "target is helper",              FATAL,       CALLSITE)
 INLINE_OBSERVATION(IS_NOT_DIRECT,             bool,   "target not direct",             FATAL,       CALLSITE)
 INLINE_OBSERVATION(IS_NOT_DIRECT_MANAGED,     bool,   "target not direct managed",     FATAL,       CALLSITE)
-INLINE_OBSERVATION(IS_RECURSIVE_OR_DEEP,      bool,   "recursive or too deep",         FATAL,       CALLSITE)
+INLINE_OBSERVATION(IS_RECURSIVE,              bool,   "recursive",                     FATAL,       CALLSITE)
+INLINE_OBSERVATION(IS_TOO_DEEP,               bool,   "too deep",                      FATAL,       CALLSITE)
 INLINE_OBSERVATION(IS_VIRTUAL,                bool,   "virtual",                       FATAL,       CALLSITE)
 INLINE_OBSERVATION(IS_VM_NOINLINE,            bool,   "noinline per VM",               FATAL,       CALLSITE)
 INLINE_OBSERVATION(IS_WITHIN_CATCH,           bool,   "within catch region",           FATAL,       CALLSITE)
@@ -134,6 +135,7 @@ INLINE_OBSERVATION(TOO_MANY_LOCALS,           bool,   "too many locals",
 
 INLINE_OBSERVATION(ARGS_OK,                   bool,   "arguments suitable",            INFORMATION, CALLSITE)
 INLINE_OBSERVATION(BENEFIT_MULTIPLIER,        double, "benefit multiplier",            INFORMATION, CALLSITE)
+INLINE_OBSERVATION(DEPTH,                     int,    "depth"             ,            INFORMATION, CALLSITE)
 INLINE_OBSERVATION(LOCALS_OK,                 bool,   "locals suitable",               INFORMATION, CALLSITE)
 INLINE_OBSERVATION(NATIVE_SIZE_ESTIMATE,      double, "native size estimate",          INFORMATION, CALLSITE)
 INLINE_OBSERVATION(NATIVE_SIZE_ESTIMATE_OK,   bool,   "native size estimate ok",       INFORMATION, CALLSITE)
index a2a060f..eb2890a 100644 (file)
@@ -7,19 +7,67 @@
 // This file contains enum and class definitions and related
 // information that the jit uses to make inlining decisions.
 //
-// -- Overview of classes and enums defined here --
+// -- ENUMS --
 //
-// InlineDecision -- enum, overall decision made about an inline
-// InlineTarget -- enum, target of a particular observation
-// InlineImpact -- enum, impact of a particular observation
-// InlineObservation -- enum, facts observed when considering an inline
-// InlineResult -- class, accumulates observations and makes a decision
-// InlineCandidateInfo -- struct, detailed information needed for inlining
-// InlArgInfo -- struct, information about a candidate's argument
-// InlLclVarInfo -- struct, information about a candidate's local variable
-// InlineHints -- enum, alternative form of observations
-// InlineInfo -- struct, basic information needed for inlining
-// InlineContext -- class, remembers what inlines happened
+// InlineDecision      - overall decision made about an inline
+// InlineTarget        - target of a particular observation
+// InlineImpact        - impact of a particular observation
+// InlineObservation   - facts observed when considering an inline
+// InlineHints         - alternative form of observations
+//
+// -- CLASSES --
+//
+// InlineResult        - accumulates observations and makes a decision
+// InlineCandidateInfo - basic information needed for inlining
+// InlArgInfo          - information about a candidate's argument
+// InlLclVarInfo       - information about a candidate's local variable
+// InlineInfo          - detailed information needed for inlining
+// InlineContext       - class, remembers what inlines happened
+// InlinePolicy        - (forthcoming)
+//
+// Enums are used throughout to provide various descriptions.
+//
+// Classes are used as follows. There are 4 sitations where inline
+// candidacy is evaluated.  In each case an InlineResult is allocated
+// on the stack to collect information about the inline candidate.
+//
+// 1. Importer Candidate Screen (impMarkInlineCandidate)
+//
+// Creates: InlineCandidateInfo
+//
+// During importing, the IL being imported is scanned to identify
+// inline candidates. This happens both when the root method is being
+// imported as well as when prospective inlines are being imported.
+// Candidates are marked in the IL and given an InlineCandidateInfo.
+//
+// 2. Inlining Optimization Pass (fgInline/fgMorphCallInline)
+//
+// Creates / Uses: InlineContext
+// Creates: InlineInfo, InlArgInfo, InlLocalVarInfo
+//
+// During the inlining optimation pass, each candidate is further
+// analyzed. Viable candidates will eventually inspire creation of an
+// InlineInfo and a set of InlArgInfos (for call arguments) and 
+// InlLocalVarInfos (for callee locals).
+//
+// The analysis will also examine InlineContexts from relevant prior
+// inlines. If the inline is successful, a new InlineContext will be
+// created to remember this inline. In DEBUG builds, failing inlines
+// also create InlineContexts.
+//
+// 3 & 4. Prejit suitability screens (compCompileHelper)
+//
+// When prejitting, each method is scanned to see if it is a viable
+// inline candidate. The scanning happens in two stages.
+//
+// A note on InlinePolicy
+//
+// In the current code base, the inlining policy is distributed across
+// the various parts of the code that drive the inlining process
+// forward. Subsequent refactoring will extract some or all of this
+// policy into a separate InlinePolicy object, to make it feasible to
+// create and experiment with alternative policies, while preserving
+// the existing policy as a baseline and fallback.
 
 #ifndef _INLINE_H_
 #define _INLINE_H_