From 2762be82aa6da467fc3423968a22fb0a48f2725f Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Wed, 22 May 2024 13:54:58 +0000
Subject: [PATCH] JIT: Compact blocks in fgMoveBackwardJumpsToSuccessors
 (#102512)

Follow-up to #102461, and part of #9304. Compacting blocks after establishing an RPO-based layout, but before moving backward jumps to fall into their successors, can enable more opportunities for branch removal; see #9304 for one such example.
---
 src/coreclr/jit/compiler.h |  2 +-
 src/coreclr/jit/fgopt.cpp  | 27 +++++++++++++++++++++++----
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index f9c66b63d15afc..d7a0086ed9bb90 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -6044,7 +6044,7 @@ class Compiler
 
     bool fgCanCompactBlocks(BasicBlock* block, BasicBlock* bNext);
 
-    void fgCompactBlocks(BasicBlock* block, BasicBlock* bNext);
+    void fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(bool doDebugCheck = true));
 
     BasicBlock* fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst);
 
diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp
index cf736d6f5d6bb9..de2b6a8f37c936 100644
--- a/src/coreclr/jit/fgopt.cpp
+++ b/src/coreclr/jit/fgopt.cpp
@@ -965,8 +965,10 @@ bool Compiler::fgCanCompactBlocks(BasicBlock* block, BasicBlock* bNext)
 // Arguments:
 //    block - move all code into this block.
 //    bNext - bbNext of `block`. This block will be removed.
+//    doDebugCheck - in Debug builds, check flowgraph for correctness after compaction
+//    (some callers might compact blocks during destructive flowgraph changes, and thus should skip checks)
 //
-void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext)
+void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(bool doDebugCheck /* = true */))
 {
     noway_assert(block != nullptr);
     noway_assert(bNext != nullptr);
@@ -1331,7 +1333,7 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext)
 #endif
 
 #if DEBUG
-    if (JitConfig.JitSlowDebugChecksEnabled() != 0)
+    if (doDebugCheck && (JitConfig.JitSlowDebugChecksEnabled() != 0))
     {
         // Make sure that the predecessor lists are accurate
         fgDebugCheckBBlist();
@@ -4555,6 +4557,23 @@ void Compiler::fgMoveBackwardJumpsToSuccessors()
     }
 #endif // DEBUG
 
+    // Compact blocks before trying to move any jumps.
+    // This can unlock more opportunities for fallthrough behavior.
+    //
+    for (BasicBlock* block = fgFirstBB; block != fgFirstFuncletBB;)
+    {
+        if (fgCanCompactBlocks(block, block->Next()))
+        {
+            // We haven't fixed EH information yet, so don't do any correctness checks here
+            //
+            fgCompactBlocks(block, block->Next() DEBUGARG(/* doDebugCheck */ false));
+        }
+        else
+        {
+            block = block->Next();
+        }
+    }
+
     EnsureBasicBlockEpoch();
     BlockSet visitedBlocks(BlockSetOps::MakeEmpty(this));
     BlockSetOps::AddElemD(this, visitedBlocks, fgFirstBB->bbNum);
@@ -4747,8 +4766,6 @@ void Compiler::fgDoReversePostOrderLayout()
         }
     }
 
-    fgMoveBackwardJumpsToSuccessors</* hasEH */ true>();
-
     // Fix up call-finally pairs
     //
     for (int i = 0; i < callFinallyPairs.Height(); i++)
@@ -4758,6 +4775,8 @@ void Compiler::fgDoReversePostOrderLayout()
         fgInsertBBafter(pair.callFinally, pair.callFinallyRet);
     }
 
+    fgMoveBackwardJumpsToSuccessors</* hasEH */ true>();
+
     // The RPO won't change the entry blocks of any EH regions, but reordering can change the last block in a region
     // (for example, by pushing throw blocks unreachable via normal flow to the end of the region).
     // First, determine the new EH region ends.