From a9c20d2ec763186650e1c0ff49c572760e5d7525 Mon Sep 17 00:00:00 2001
From: Joseph Tremoulet <jotrem@microsoft.com>
Date: Thu, 17 Aug 2017 14:54:38 -0400
Subject: [PATCH] Check all preds for each `top`

Since we may treat multiple back-edges to the same successor as nested
loops, we need to continue iterating a `top` block's predecessors even
after finding a back-edge targeting it.
---
 src/jit/optimizer.cpp | 103 ++++++++++++++++++------------------------
 1 file changed, 44 insertions(+), 59 deletions(-)

diff --git a/src/jit/optimizer.cpp b/src/jit/optimizer.cpp
index 3c43452b7bc4..7e269b3affe9 100644
--- a/src/jit/optimizer.cpp
+++ b/src/jit/optimizer.cpp
@@ -1570,43 +1570,6 @@ class LoopSearch
         assert(comp->fgBBNumMax <= comp->fgCurBBEpochSize);
     }
 
-    //------------------------------------------------------------------------
-    // findLoop: Search for a loop with the given HEAD block.
-    //
-    // Arguments:
-    //    head - Block to be the HEAD of any loop identified
-    //
-    // Return Value:
-    //    true  - Found a valid loop.
-    //    false - Did not find a valid loop.
-    //
-    // Notes:
-    //    May modify flow graph go make loop compact before returning.
-    //    Will set instance fields to track loop's extent and exits.
-    //
-    bool findLoop(BasicBlock* head)
-    {
-        BasicBlock* top = head->bbNext;
-
-        //  Blocks that are rarely run have a zero bbWeight and should
-        //  never be optimized here
-
-        if (top->bbWeight == BB_ZERO_WEIGHT)
-        {
-            return false;
-        }
-
-        for (flowList* pred = top->bbPreds; pred; pred = pred->flNext)
-        {
-            if (findLoop(head, top, pred->flBlock))
-            {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
     //------------------------------------------------------------------------
     // recordLoop: Notify the Compiler that a loop has been found.
     //
@@ -1633,7 +1596,6 @@ class LoopSearch
         return false;
     }
 
-private:
     //------------------------------------------------------------------------
     // findLoop: Search for a loop with the given HEAD block and back-edge.
     //
@@ -1778,6 +1740,7 @@ class LoopSearch
         return true;
     }
 
+private:
     //------------------------------------------------------------------------
     // findEntry: See if given HEAD flows to valid ENTRY between given TOP and BOTTOM
     //
@@ -2420,37 +2383,59 @@ void Compiler::optFindNaturalLoops()
 
     for (BasicBlock* head = fgFirstBB; head->bbNext; head = head->bbNext)
     {
-        if (search.findLoop(head))
+        BasicBlock* top = head->bbNext;
+
+        //  Blocks that are rarely run have a zero bbWeight and should
+        //  never be optimized here
+
+        if (top->bbWeight == BB_ZERO_WEIGHT)
         {
-            // Found a loop; record it and see if we've hit the limit.
-            bool recordedLoop = search.recordLoop();
+            continue;
+        }
 
-            (recordedLoop); // avoid unusued variable warnings in COUNT_LOOPS and !DEBUG
+        for (flowList* pred = top->bbPreds; pred; pred = pred->flNext)
+        {
+            if (search.findLoop(head, top, pred->flBlock))
+            {
+                // Found a loop; record it and see if we've hit the limit.
+                bool recordedLoop = search.recordLoop();
+
+                (recordedLoop); // avoid unusued variable warnings in COUNT_LOOPS and !DEBUG
 
 #if COUNT_LOOPS
-            if (!hasMethodLoops)
-            {
-                /* mark the method as containing natural loops */
-                totalLoopMethods++;
-                hasMethodLoops = true;
-            }
+                if (!hasMethodLoops)
+                {
+                    /* mark the method as containing natural loops */
+                    totalLoopMethods++;
+                    hasMethodLoops = true;
+                }
 
-            /* increment total number of loops found */
-            totalLoopCount++;
-            loopsThisMethod++;
+                /* increment total number of loops found */
+                totalLoopCount++;
+                loopsThisMethod++;
 
-            /* keep track of the number of exits */
-            loopExitCountTable.record(static_cast<unsigned>(exitCount));
+                /* keep track of the number of exits */
+                loopExitCountTable.record(static_cast<unsigned>(exitCount));
 #else  // COUNT_LOOPS
-            assert(recordedLoop);
-            if (optLoopCount == MAX_LOOP_NUM)
-            {
-                // We won't be able to record any more loops, so stop looking.
-                break;
-            }
+                assert(recordedLoop);
+                if (optLoopCount == MAX_LOOP_NUM)
+                {
+                    // We won't be able to record any more loops, so stop looking.
+                    goto NO_MORE_LOOPS;
+                }
 #endif // COUNT_LOOPS
+
+                // Continue searching preds of `top` to see if any other are
+                // back-edges (this can happen for nested loops).  The iteration
+                // is safe because the compaction we do only modifies predecessor
+                // lists of blocks that gain or lose fall-through from their
+                // `bbPrev`, but since the motion is from within the loop to below
+                // it, we know we're not altering the relationship between `top`
+                // and its `bbPrev`.
+            }
         }
     }
+NO_MORE_LOOPS:
 
 #if COUNT_LOOPS
     loopCountTable.record(loopsThisMethod);