From 15e5a70bca87dd0cba0fc9dff87d5eb298c43802 Mon Sep 17 00:00:00 2001
From: Andy Ayers <andya@microsoft.com>
Date: Fri, 29 Mar 2024 10:33:51 -0700
Subject: [PATCH] JIT: fix count reconstruction when a natural loop contains an
 improper loop

If a natural loop contains an improper loop, the cyclic probability computation
for the natural loop will be an understimate, as the cyclic probability computation
assumes one pass convergence.

In such cases count reconstruction may report convergence when it has not in
fact converged, as any natural loop header ignores flow from its backedges, assuming
their impact has been accounted for by the cyclic probability.

So when a loop contains improper loops, fall back to normal iterative computation
for the loop. We could use the cyclic probability initially as a convergence
accelerator, but would need to switch over to not using it to guarantee full
convergence. But that complicates the logic and these cases are rare.
---
 src/coreclr/jit/compiler.h             | 8 ++++++++
 src/coreclr/jit/fgprofilesynthesis.cpp | 6 ++++--
 src/coreclr/jit/flowgraph.cpp          | 9 +++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index c0a60c66ad76bc..07431d8a14e803 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -2091,6 +2091,9 @@ class FlowGraphNaturalLoop
     // Can be used to store additional annotations for this loop on the side.
     unsigned m_index = 0;
 
+    // True if this loop contains an improper loop header
+    bool m_containsImproperHeader = false;
+
     FlowGraphNaturalLoop(const FlowGraphDfsTree* dfsTree, BasicBlock* head);
 
     unsigned LoopBlockBitVecIndex(BasicBlock* block);
@@ -2179,6 +2182,11 @@ class FlowGraphNaturalLoop
     bool ContainsBlock(BasicBlock* block);
     bool ContainsLoop(FlowGraphNaturalLoop* childLoop);
 
+    bool ContainsImproperHeader() const
+    {
+        return m_containsImproperHeader;
+    }
+
     unsigned NumLoopBlocks();
 
     template<typename TFunc>
diff --git a/src/coreclr/jit/fgprofilesynthesis.cpp b/src/coreclr/jit/fgprofilesynthesis.cpp
index 2de615136cfeb4..0ab8576cb24b86 100644
--- a/src/coreclr/jit/fgprofilesynthesis.cpp
+++ b/src/coreclr/jit/fgprofilesynthesis.cpp
@@ -1193,12 +1193,14 @@ void ProfileSynthesis::GaussSeidelSolver()
             //
             if (block->bbPreds != nullptr)
             {
-                // Leverage Cp for existing loop headers.
+                // Leverage Cp for existing loop headers, provided that
+                // all contained loops are proper.
+                //
                 // This is an optimization to speed convergence.
                 //
                 FlowGraphNaturalLoop* const loop = m_loops->GetLoopByHeader(block);
 
-                if (loop != nullptr)
+                if ((loop != nullptr) && !loop->ContainsImproperHeader())
                 {
                     // Sum all entry edges that aren't EH flow
                     //
diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp
index 9b9f3a3f951418..e2f450a7cb194d 100644
--- a/src/coreclr/jit/flowgraph.cpp
+++ b/src/coreclr/jit/flowgraph.cpp
@@ -4386,6 +4386,15 @@ FlowGraphNaturalLoops* FlowGraphNaturalLoops::Find(const FlowGraphDfsTree* dfsTr
         if (!FindNaturalLoopBlocks(loop, worklist))
         {
             loops->m_improperLoopHeaders++;
+
+            for (FlowGraphNaturalLoop* const otherLoop : loops->InPostOrder())
+            {
+                if (otherLoop->ContainsBlock(header))
+                {
+                    otherLoop->m_containsImproperHeader = true;
+                }
+            }
+
             continue;
         }