JIT: only tail recursive calls become loops (dotnet/coreclr#27079)
authorAndy Ayers <andya@microsoft.com>
Thu, 10 Oct 2019 06:46:23 +0000 (23:46 -0700)
committerGitHub <noreply@github.com>
Thu, 10 Oct 2019 06:46:23 +0000 (23:46 -0700)
Importer was a bit too liberal marking recursive calls as loop-inducing, at
least from the standpoint of enforcing backward-branch constraints. This would
sometimes lead us to inline into cold paths like throws. Only tail-recursive
calls will turn into loops.

Also, future-proof the propagation of simple loopness the root compiler, in
case we ever decide to enable inlining at Tier0.

Commit migrated from https://github.com/dotnet/coreclr/commit/6b13065e7ac88671795a01577e017ff7cdc26f35

src/coreclr/src/jit/compiler.cpp
src/coreclr/src/jit/compiler.h
src/coreclr/src/jit/flowgraph.cpp
src/coreclr/src/jit/importer.cpp

index d9aafbc..bc604f5 100644 (file)
@@ -5776,7 +5776,7 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE            classPtr,
     info.compTotalHotCodeSize  = 0;
     info.compTotalColdCodeSize = 0;
 
-    fgHasBackwardJump = false;
+    compHasBackwardJump = false;
 
 #ifdef DEBUG
     compCurBB = nullptr;
@@ -5912,10 +5912,10 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE            classPtr,
     }
 
 #ifdef FEATURE_CORECLR
-    if (fgHasBackwardJump && (info.compFlags & CORINFO_FLG_DISABLE_TIER0_FOR_LOOPS) != 0 && fgCanSwitchToOptimized())
+    if (compHasBackwardJump && (info.compFlags & CORINFO_FLG_DISABLE_TIER0_FOR_LOOPS) != 0 && fgCanSwitchToOptimized())
 #else // !FEATURE_CORECLR
     // We may want to use JitConfig value here to support DISABLE_TIER0_FOR_LOOPS
-    if (fgHasBackwardJump && fgCanSwitchToOptimized())
+    if (compHasBackwardJump && fgCanSwitchToOptimized())
 #endif
     {
         // Method likely has a loop, switch to the OptimizedTier to avoid spending too much time running slower code
index 2813625..2907685 100644 (file)
@@ -5183,8 +5183,6 @@ protected:
     void        fgInitBBLookup();
     BasicBlock* fgLookupBB(unsigned addr);
 
-    bool fgHasBackwardJump;
-
     bool fgCanSwitchToOptimized();
     void fgSwitchToOptimized();
 
@@ -8241,6 +8239,7 @@ public:
     bool compQmarkUsed;            // Does the method use GT_QMARK/GT_COLON
     bool compQmarkRationalized;    // Is it allowed to use a GT_QMARK/GT_COLON node.
     bool compUnsafeCastUsed;       // Does the method use LDIND/STIND to cast between scalar/refernce types
+    bool compHasBackwardJump;      // Does the method (or some inlinee) have a lexically backwards jump?
 
 // NOTE: These values are only reliable after
 //       the importing is completely finished.
@@ -8262,7 +8261,7 @@ public:
     bool compLSRADone;
     bool compRationalIRForm;
 
-    bool compUsesThrowHelper; // There is a call to a THOROW_HELPER for the compiled method.
+    bool compUsesThrowHelper; // There is a call to a THROW_HELPER for the compiled method.
 
     bool compGeneratingProlog;
     bool compGeneratingEpilog;
index f23cdf2..9dcf9f6 100644 (file)
@@ -4310,6 +4310,7 @@ void Compiler::fgSwitchToOptimized()
     assert(fgCanSwitchToOptimized());
 
     // Switch to optimized and re-init options
+    JITDUMP("****\n**** JIT Tier0 jit request switching to Tier1 because of loop\n****\n");
     assert(opts.jitFlags->IsSet(JitFlags::JIT_FLAG_TIER0));
     opts.jitFlags->Clear(JitFlags::JIT_FLAG_TIER0);
     compInitOptions(opts.jitFlags);
@@ -5238,7 +5239,7 @@ void Compiler::fgMarkBackwardJump(BasicBlock* startBlock, BasicBlock* endBlock)
         if ((block->bbFlags & BBF_BACKWARD_JUMP) == 0)
         {
             block->bbFlags |= BBF_BACKWARD_JUMP;
-            fgHasBackwardJump = true;
+            compHasBackwardJump = true;
         }
     }
 }
@@ -23178,6 +23179,7 @@ _Done:
     compUnsafeCastUsed |= InlineeCompiler->compUnsafeCastUsed;
     compNeedsGSSecurityCookie |= InlineeCompiler->compNeedsGSSecurityCookie;
     compGSReorderStackLayout |= InlineeCompiler->compGSReorderStackLayout;
+    compHasBackwardJump |= InlineeCompiler->compHasBackwardJump;
 
 #ifdef FEATURE_SIMD
     if (InlineeCompiler->usesSIMDTypes())
index 6ace7a4..65261e9 100644 (file)
@@ -7542,20 +7542,6 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
         exactContextHnd                = callInfo->contextHandle;
         exactContextNeedsRuntimeLookup = callInfo->exactContextNeedsRuntimeLookup == TRUE;
 
-        // Recursive call is treated as a loop to the begining of the method.
-        if (gtIsRecursiveCall(methHnd))
-        {
-#ifdef DEBUG
-            if (verbose)
-            {
-                JITDUMP("\nFound recursive call in the method. Mark " FMT_BB " to " FMT_BB
-                        " as having a backward branch.\n",
-                        fgFirstBB->bbNum, compCurBB->bbNum);
-            }
-#endif
-            fgMarkBackwardJump(fgFirstBB, compCurBB);
-        }
-
         switch (callInfo->kind)
         {
 
@@ -8482,6 +8468,15 @@ DONE:
         }
     }
 
+    // A tail recursive call is a potential loop from the current block to the start of the method.
+    if (canTailCall && gtIsRecursiveCall(methHnd))
+    {
+        JITDUMP("\nFound tail recursive call in the method. Mark " FMT_BB " to " FMT_BB
+                " as having a backward branch.\n",
+                fgFirstBB->bbNum, compCurBB->bbNum);
+        fgMarkBackwardJump(fgFirstBB, compCurBB);
+    }
+
     // Note: we assume that small return types are already normalized by the managed callee
     // or by the pinvoke stub for calls to unmanaged code.