From 9086686eef3cc156f07085791b82115a379cfadb Mon Sep 17 00:00:00 2001
From: Bruce Forstall <brucefo@microsoft.com>
Date: Thu, 13 Oct 2022 12:14:25 -0700
Subject: [PATCH] Fix insertion of alignment after BBJ_CALLFINALLY/BBJ_ALWAYS
 (#76988)

The Runtime_76346 test exposed a case where, in the case of STRESS_EMITTER,
we were inserting breakpoint instructions instead of NOPs for loop alignment
when the alignment followed an unconditional branch. However, it wasn't
considering the case of a BBJ_CALLFINALLY/BBJ_ALWAYS pair immediately
followed by a loop head.

This pointed out that alignment instructions should never be inserted in that
BBJ_ALWAYS block, since that block shouldn't contain any code. Inserting the
alignment NOPs affected the reported range of the EH cloned finally region
in the FEATURE_EH_CALLFINALLY_THUNKS case.

In these special cases, we simply give up on trying to align the loop.

No diffs.

Fixes #76910
---
 src/coreclr/jit/codegenlinear.cpp |  5 ++++
 src/coreclr/jit/compiler.cpp      | 38 +++++++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp
index da8cae58d0c28a..db0e588bc56564 100644
--- a/src/coreclr/jit/codegenlinear.cpp
+++ b/src/coreclr/jit/codegenlinear.cpp
@@ -823,7 +823,12 @@ void CodeGen::genCodeForBBlist()
             // compJitAlignLoopBoundary.
             // For adaptive alignment, alignment instruction will always be of 15 bytes for xarch
             // and 16 bytes for arm64.
+
             assert(ShouldAlignLoops());
+            assert(!block->isBBCallAlwaysPairTail());
+#if FEATURE_EH_CALLFINALLY_THUNKS
+            assert(block->bbJumpKind != BBJ_CALLFINALLY);
+#endif // FEATURE_EH_CALLFINALLY_THUNKS
 
             GetEmitter()->emitLoopAlignment(DEBUG_ARG1(block->bbJumpKind == BBJ_ALWAYS));
         }
diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp
index de98d3c2f160cb..ce74fbb1d95012 100644
--- a/src/coreclr/jit/compiler.cpp
+++ b/src/coreclr/jit/compiler.cpp
@@ -5090,14 +5090,44 @@ PhaseStatus Compiler::placeLoopAlignInstructions()
             BasicBlock* const loopTop = block->bbNext;
 
             // If jmp was not found, then block before the loop start is where align instruction will be added.
+            // There are two special cases:
+            // 1. If the block before the loop start is a retless BBJ_CALLFINALLY with
+            //    FEATURE_EH_CALLFINALLY_THUNKS, we can't add alignment because it will affect reported EH
+            //    region range.
+            // 2. If the previous block is the BBJ_ALWAYS of a BBJ_CALLFINALLY/BBJ_ALWAYS pair, then we
+            //    can't add alignment because we can't add instructions in that block. In the
+            //    FEATURE_EH_CALLFINALLY_THUNKS case, it would affect the reported EH, as above.
+            //
+            // Currently, we don't align loops for these cases.
             //
             if (bbHavingAlign == nullptr)
             {
-                // In some odd cases we may see blocks within the loop before we see the
-                // top block of the loop. Just bail on aligning such loops.
-                //
-                if ((block->bbNatLoopNum != BasicBlock::NOT_IN_LOOP) && (block->bbNatLoopNum == loopTop->bbNatLoopNum))
+                bool isSpecialCallFinally = block->isBBCallAlwaysPairTail();
+#if FEATURE_EH_CALLFINALLY_THUNKS
+                if (block->bbJumpKind == BBJ_CALLFINALLY)
+                {
+                    // It must be a retless BBJ_CALLFINALLY if we get here.
+                    assert(!block->isBBCallAlwaysPair());
+
+                    // In the case of FEATURE_EH_CALLFINALLY_THUNKS, we can't put the align instruction in a retless
+                    // BBJ_CALLFINALLY either, because it alters the "cloned finally" region reported to the VM.
+                    // In the x86 case (the only !FEATURE_EH_CALLFINALLY_THUNKS that supports retless
+                    // BBJ_CALLFINALLY), we allow it.
+                    isSpecialCallFinally = true;
+                }
+#endif // FEATURE_EH_CALLFINALLY_THUNKS
+
+                if (isSpecialCallFinally)
+                {
+                    loopTop->unmarkLoopAlign(this DEBUG_ARG("block before loop is special callfinally/always block"));
+                    madeChanges = true;
+                }
+                else if ((block->bbNatLoopNum != BasicBlock::NOT_IN_LOOP) &&
+                         (block->bbNatLoopNum == loopTop->bbNatLoopNum))
                 {
+                    // In some odd cases we may see blocks within the loop before we see the
+                    // top block of the loop. Just bail on aligning such loops.
+                    //
                     loopTop->unmarkLoopAlign(this DEBUG_ARG("loop block appears before top of loop"));
                     madeChanges = true;
                 }