Inline refactoring: rework logic in compCompileHelper

This change reworks the logic in `compCompileHelper` to split
out the invocation of `fgFindBasicBlocks` for the special
prejit-root case (see dotnet/coreclr#3371). As a side benefit the two InlineResults
needed for the prejit inline scan can be merged.

Commit migrated from https://github.com/dotnet/coreclr/commit/026a933121c89c823fb7ebf91dd74eeb456608b2

diff --git a/src/coreclr/src/jit/compiler.cpp b/src/coreclr/src/jit/compiler.cpp
index 2642866..3781b67 100644 (file)
--- a/src/coreclr/src/jit/compiler.cpp
+++ b/src/coreclr/src/jit/compiler.cpp
@@ -4924,80 +4924,83 @@ int           Compiler::compCompileHelper (CORINFO_MODULE_HANDLE            clas
 
         lvaInitTypeRef();
 
-        bool hasBeenMarkedAsBadInlinee = false;
-        bool forceInline = !!(info.compFlags & CORINFO_FLG_FORCEINLINE);
-
         if (!compIsForInlining())
         {
             compInitDebuggingInfo();
-
-            if (opts.eeFlags & CORJIT_FLG_PREJIT)
-            {
-                // Cache inlining hint during NGen to avoid touching bodies of non-inlineable methods at runtime
-                InlineResult trialResult(this, methodHnd, "prejit1");
-                impCanInlineIL(methodHnd, methodInfo, forceInline, &trialResult);
-                if (trialResult.isFailure())
-                {
-                    // It is a bad inlinee according to impCanInlineIL.
-                    // This decision better not be context-dependent.
-                    assert(trialResult.isNever());
-
-                    // Don't bother with the second stage of the evaluation for this method.
-                    hasBeenMarkedAsBadInlinee = true;
-                }
-                else
-                {
-                    // Since we're not actually inlining anything, don't report success.
-                    trialResult.setReported();
-                }
-            }
         }
 
-        /* Find and create the basic blocks */
+        const bool forceInline = !!(info.compFlags & CORINFO_FLG_FORCEINLINE);
 
-        fgFindBasicBlocks();        
-        if (compDonotInline())
+        if (!compIsForInlining() && (opts.eeFlags & CORJIT_FLG_PREJIT))
         {
-            goto _Next;
-        }          
+            // We're prejitting the root method. We also will analyze it as
+            // a potential inline candidate.
+            InlineResult prejitResult(this, methodHnd, "prejit");
 
-        //
-        // Now, we might have calculated the compNativeSizeEstimate in fgFindJumpTargets.
-        // If we haven't marked this method as a bad inlinee as a result of impCanInlineIL, 
-        // check to see if it is a bad inlinee according to impCanInlineNative.
-        //        
-        if (!compIsForInlining()                       && // We are compiling a method (not inlining one).
-            !hasBeenMarkedAsBadInlinee                 && // The method hasn't been marked as bad inlinee.
-            (opts.eeFlags & CORJIT_FLG_PREJIT)         && // This is NGEN.
-            !forceInline                               && // The size of the method matters.
-            (methodInfo->ILCodeSize > ALWAYS_INLINE_SIZE))
-        {
-            assert(methodInfo->ILCodeSize <= impInlineSize); // Otherwise it must have been marked as a bad inlinee by impCanInlineIL. 
-
-            // We must have run the CodeSeq state machine and got the native size estimate.
-            assert(compNativeSizeEstimate != NATIVE_SIZE_INVALID); 
-
-            int callsiteNativeSizeEstimate = impEstimateCallsiteNativeSize(methodInfo);
-            InlineResult trialResult(this, methodHnd, "prejit2");
-            
-            impCanInlineNative(callsiteNativeSizeEstimate, 
-                               compNativeSizeEstimate,
-                               compInlineeHints,
-                               nullptr, // Calculate static inlining hint.
-                               &trialResult);
+            // Do the initial inline screen.
+            impCanInlineIL(methodHnd, methodInfo, forceInline, &prejitResult);
 
-            if (trialResult.isFailure())
+            // Find the basic blocks. We must do this regardless of
+            // inlineability, since we are prejitting this method.
+            //
+            // Among other things, this will set
+            // compNativeSizeEstimate and compInlineeHints for the
+            // subset of methods we check below.
+            fgFindBasicBlocks();
+
+            // If this method is still a viable inline candidate,
+            // do the profitability screening.
+            if (prejitResult.isCandidate())
+            {
+                // Only needed if the inline is discretionary (not forced)
+                // and the size is over the always threshold.
+                if (!forceInline && (methodInfo->ILCodeSize > ALWAYS_INLINE_SIZE))
+                {
+                    // We should have run the CodeSeq state machine
+                    // and got the native size estimate.
+                    assert(compNativeSizeEstimate != NATIVE_SIZE_INVALID);
+
+                    // Estimate the call site impact
+                    int callsiteNativeSizeEstimate = impEstimateCallsiteNativeSize(methodInfo);
+
+                    // See if we're willing to pay for inlining this method
+                    impCanInlineNative(callsiteNativeSizeEstimate,
+                                       compNativeSizeEstimate,
+                                       compInlineeHints,
+                                       nullptr, // Calculate static inlining hint.
+                                       &prejitResult);
+                }
+            }
+
+            // Handle the results of the inline analysis.
+            if (prejitResult.isFailure())
             {
-                // Bingo! It is a bad inlinee according to impCanInlineNative.
+                // This method is a bad inlinee according to our
+                // analysis.  We will let the InlineResult destructor
+                // mark it as noinline in the prejit image to save the
+                // jit some work.
+                //
                 // This decision better not be context-dependent.
-                assert(trialResult.isNever());
+                assert(prejitResult.isNever());
             }
-            else 
+            else
             {
-               // Since we're not actually inlining, don't report success.
-               trialResult.setReported();
+                // This looks like a viable inline candidate.  Since
+                // we're not actually inlining, don't report anything.
+                prejitResult.setReported();
             }
         }
+        else
+        {
+            // We are jitting the root method, or inlining.
+            fgFindBasicBlocks();
+        }
+
+        // If we're inlining and the candidate is bad, bail out.
+        if (compDonotInline())
+        {
+            goto _Next;
+        }          
 
         compSetOptimizationLevel();
 

diff --git a/src/coreclr/src/jit/inline.h b/src/coreclr/src/jit/inline.h
index 5a4012b..4d24383 100644 (file)
--- a/src/coreclr/src/jit/inline.h
+++ b/src/coreclr/src/jit/inline.h
@@ -63,10 +63,10 @@
 // In DEBUG, the jit also searches for non-candidate calls to try
 // and get a complete picture of the set of failed inlines.
 //
-// 4 & 5. Prejit suitability screens (compCompileHelper)
+// 4. Prejit suitability screen (compCompileHelper)
 //
 // When prejitting, each method is scanned to see if it is a viable
-// inline candidate. The scanning happens in two stages.
+// inline candidate.
 //
 // A note on InlinePolicy
 //