Fix phase order problem with throw helper blocks (#97201)

Throw helper blocks are created in morph, then possibly removed if
unnecessary in StackLevelSetter (under optimization). However, there
was a case where StackLevelSetter removed an OVERFLOW throw helper
block after optimization proved it unnecessary because of a constant
zero dividend, but between StackLevelSetter and codegen, LSRA introduced
a RELOAD node above the constant zero that `GenTree::CanDivOrModPossiblyOverflow()`
didn't understand, thus causing it to think that overflow was possible.
Codegen looked for the OVERFLOW throw helper block and couldn't find it.

There are multiple fixes here, somewhat "defense in depth":
- If `StackLevelSetter::SetThrowHelperBlocks()` determines a node can't throw
divide-by-zero or ArithmeticException (overflow), it marks the node
GTF_DIV_MOD_NO_BY_ZERO / GTF_DIV_MOD_NO_OVERFLOW, respectively. This is what
morph does earlier in compilation.
- `genMarkLabelsForCodegen()` does not mark throw helper blocks where `acdUsed`
is false, to avoid marking deleted blocks.
- More asserts are added that `acdUsed` is true when codegen goes to generate
a branch to a throw helper.
- `GenTree::OperExceptions` / `CanDivOrModPossiblyOverflow` are changed to skip
COPY/RELOAD nodes.

Fixes #96224

src/coreclr/jit/codegencommon.cpp

@@ -455,10 +455,13 @@ void CodeGen::genMarkLabelsForCodegen()
     }
 
     // Walk all the exceptional code blocks and mark them, since they don't appear in the normal flow graph.
-    for (Compiler::AddCodeDsc* add = compiler->fgAddCodeList; add; add = add->acdNext)
+    for (Compiler::AddCodeDsc* add = compiler->fgAddCodeList; add != nullptr; add = add->acdNext)
     {
-        JITDUMP("  " FMT_BB " : throw helper block\n", add->acdDstBlk->bbNum);
-        add->acdDstBlk->SetFlags(BBF_HAS_LABEL);
+        if (add->acdUsed)
+        {
+            JITDUMP("  " FMT_BB " : throw helper block\n", add->acdDstBlk->bbNum);
+            add->acdDstBlk->SetFlags(BBF_HAS_LABEL);
+        }
     }
 
     for (EHblkDsc* const HBtab : EHClauses(compiler))
@@ -1521,6 +1524,7 @@ void CodeGen::genJumpToThrowHlpBlk(emitJumpKind jumpKind, SpecialCodeKind codeKi
 #ifdef DEBUG
             Compiler::AddCodeDsc* add =
                 compiler->fgFindExcptnTarget(codeKind, compiler->bbThrowIndex(compiler->compCurBB));
+            assert(add->acdUsed);
             assert(excpRaisingBlock == add->acdDstBlk);
 #if !FEATURE_FIXED_OUT_ARGS
             assert(add->acdStkLvlInit || isFramePointerUsed());
@@ -1533,6 +1537,7 @@ void CodeGen::genJumpToThrowHlpBlk(emitJumpKind jumpKind, SpecialCodeKind codeKi
             Compiler::AddCodeDsc* add =
                 compiler->fgFindExcptnTarget(codeKind, compiler->bbThrowIndex(compiler->compCurBB));
             PREFIX_ASSUME_MSG((add != nullptr), ("ERROR: failed to find exception throw block"));
+            assert(add->acdUsed);
             excpRaisingBlock = add->acdDstBlk;
 #if !FEATURE_FIXED_OUT_ARGS
             assert(add->acdStkLvlInit || isFramePointerUsed());

src/coreclr/jit/codegenloongarch64.cpp

@@ -7671,6 +7671,7 @@ inline void CodeGen::genJumpToThrowHlpBlk_la(
 #ifdef DEBUG
             Compiler::AddCodeDsc* add =
                 compiler->fgFindExcptnTarget(codeKind, compiler->bbThrowIndex(compiler->compCurBB));
+            assert(add->acdUsed);
             assert(excpRaisingBlock == add->acdDstBlk);
 #if !FEATURE_FIXED_OUT_ARGS
             assert(add->acdStkLvlInit || isFramePointerUsed());
@@ -7683,6 +7684,7 @@ inline void CodeGen::genJumpToThrowHlpBlk_la(
             Compiler::AddCodeDsc* add =
                 compiler->fgFindExcptnTarget(codeKind, compiler->bbThrowIndex(compiler->compCurBB));
             PREFIX_ASSUME_MSG((add != nullptr), ("ERROR: failed to find exception throw block"));
+            assert(add->acdUsed);
             excpRaisingBlock = add->acdDstBlk;
 #if !FEATURE_FIXED_OUT_ARGS
             assert(add->acdStkLvlInit || isFramePointerUsed());

src/coreclr/jit/codegenriscv64.cpp

@@ -7186,6 +7186,7 @@ inline void CodeGen::genJumpToThrowHlpBlk_la(
 #ifdef DEBUG
             Compiler::AddCodeDsc* add =
                 compiler->fgFindExcptnTarget(codeKind, compiler->bbThrowIndex(compiler->compCurBB));
+            assert(add->acdUsed);
             assert(excpRaisingBlock == add->acdDstBlk);
 #if !FEATURE_FIXED_OUT_ARGS
             assert(add->acdStkLvlInit || isFramePointerUsed());
@@ -7198,6 +7199,7 @@ inline void CodeGen::genJumpToThrowHlpBlk_la(
             Compiler::AddCodeDsc* add =
                 compiler->fgFindExcptnTarget(codeKind, compiler->bbThrowIndex(compiler->compCurBB));
             PREFIX_ASSUME_MSG((add != nullptr), ("ERROR: failed to find exception throw block"));
+            assert(add->acdUsed);
             excpRaisingBlock = add->acdDstBlk;
 #if !FEATURE_FIXED_OUT_ARGS
             assert(add->acdStkLvlInit || isFramePointerUsed());

src/coreclr/jit/compiler.h

@@ -6566,7 +6566,11 @@ class Compiler
     struct AddCodeDsc
     {
         AddCodeDsc*     acdNext;
-        BasicBlock*     acdDstBlk; // block  to  which we jump
+
+        // Initially the source block of the exception. After fgCreateThrowHelperBlocks, the block to which
+        // we jump to raise the exception.
+        BasicBlock*     acdDstBlk;
+
         unsigned        acdData;
         SpecialCodeKind acdKind; // what kind of a special block is this?
         bool            acdUsed; // do we need to keep this helper block?

src/coreclr/jit/compiler.hpp

@@ -3045,7 +3045,7 @@ inline bool Compiler::fgIsThrowHlpBlk(BasicBlock* block)
     // under stress, with implausible flow graph optimizations. So, walk the fgAddCodeList
     // for the final determination.
 
-    for (AddCodeDsc* add = fgAddCodeList; add; add = add->acdNext)
+    for (AddCodeDsc* add = fgAddCodeList; add != nullptr; add = add->acdNext)
     {
         if (block == add->acdDstBlk)
         {
@@ -3068,7 +3068,7 @@ inline bool Compiler::fgIsThrowHlpBlk(BasicBlock* block)
 
 inline unsigned Compiler::fgThrowHlpBlkStkLevel(BasicBlock* block)
 {
-    for (AddCodeDsc* add = fgAddCodeList; add; add = add->acdNext)
+    for (AddCodeDsc* add = fgAddCodeList; add != nullptr; add = add->acdNext)
     {
         if (block == add->acdDstBlk)
         {

src/coreclr/jit/emit.cpp

@@ -4299,7 +4299,15 @@ void emitter::emitDispJumpList()
             else
 #endif // TARGET_ARM64
             {
-                printf(" -> IG%02u", ((insGroup*)emitCodeGetCookie(jmp->idAddr()->iiaBBlabel))->igNum);
+                insGroup* targetGroup = (insGroup*)emitCodeGetCookie(jmp->idAddr()->iiaBBlabel);
+                if (targetGroup == nullptr)
+                {
+                    printf(" -> ILLEGAL");
+                }
+                else
+                {
+                    printf(" -> IG%02u", targetGroup->igNum);
+                }
             }
 
             if (jmp->idjShort)

src/coreclr/jit/flowgraph.cpp

@@ -3539,15 +3539,15 @@ PhaseStatus Compiler::fgCreateThrowHelperBlocks()
 }
 
 //------------------------------------------------------------------------
-// fgFindExcptnTarget: finds the block to jump that will throw a given kind of exception
+// fgFindExcptnTarget: finds the block to jump to that will throw a given kind of exception
 //
 // Arguments:
-//    kind - kind of exception to throw
+//    kind -- kind of exception to throw
 //    refData -- bbThrowIndex of the block that will jump to the throw helper
 //
 // Return Value:
 //    Code descriptor for the appropriate throw helper block, or nullptr if no such
-//    descriptor exists
+//    descriptor exists.
 //
 Compiler::AddCodeDsc* Compiler::fgFindExcptnTarget(SpecialCodeKind kind, unsigned refData)
 {
@@ -3559,7 +3559,7 @@ Compiler::AddCodeDsc* Compiler::fgFindExcptnTarget(SpecialCodeKind kind, unsigne
 
     if (add == nullptr)
     {
-        // We should't be asking for these blocks late in compilation
+        // We shouldn't be asking for these blocks late in compilation
         // unless we know there are entries to be found.
         assert(!fgRngChkThrowAdded);
     }

src/coreclr/jit/gentree.cpp

@@ -6908,7 +6908,6 @@ bool GenTree::OperIsImplicitIndir() const
 //------------------------------------------------------------------------------
 // OperExceptions: Get exception set this tree may throw.
 //
-//
 // Arguments:
 //    comp      -  Compiler instance
 //
@@ -6945,11 +6944,9 @@ ExceptionSetFlags GenTree::OperExceptions(Compiler* comp)
 
             ExceptionSetFlags exSetFlags = ExceptionSetFlags::None;
 
-            GenTree* op2 = this->gtGetOp2();
-
-            if (!(this->gtFlags & GTF_DIV_MOD_NO_BY_ZERO) && !op2->IsNeverZero())
+            if (!(this->gtFlags & GTF_DIV_MOD_NO_BY_ZERO) && !this->gtGetOp2()->gtSkipReloadOrCopy()->IsNeverZero())
             {
-                exSetFlags = ExceptionSetFlags::DivideByZeroException;
+                exSetFlags |= ExceptionSetFlags::DivideByZeroException;
             }
 
             if (this->OperIs(GT_DIV, GT_MOD) && this->CanDivOrModPossiblyOverflow(comp))
@@ -27217,8 +27214,8 @@ bool GenTree::CanDivOrModPossiblyOverflow(Compiler* comp) const
     if (this->gtFlags & GTF_DIV_MOD_NO_OVERFLOW)
         return false;
 
-    GenTree* op1 = this->gtGetOp1();
-    GenTree* op2 = this->gtGetOp2();
+    GenTree* op1 = this->gtGetOp1()->gtSkipReloadOrCopy();
+    GenTree* op2 = this->gtGetOp2()->gtSkipReloadOrCopy();
 
     // If the divisor is known to never be '-1', we cannot overflow.
     if (op2->IsNeverNegativeOne(comp))

src/coreclr/jit/gentree.h

@@ -8954,7 +8954,6 @@ inline bool GenTree::OperIsCopyBlkOp()
 //    long constants in a target-independent way.
 
 inline bool GenTree::IsIntegralConst(ssize_t constVal) const
-
 {
     if ((gtOper == GT_CNS_INT) && (AsIntConCommon()->IconValue() == constVal))
     {
@@ -9372,9 +9371,9 @@ inline GenTree* GenTree::gtCommaStoreVal()
 inline GenTree* GenTree::gtSkipReloadOrCopy()
 {
     // There can be only one reload or copy (we can't have a reload/copy of a reload/copy)
-    if (gtOper == GT_RELOAD || gtOper == GT_COPY)
+    if (OperIs(GT_RELOAD, GT_COPY))
     {
-        assert(gtGetOp1()->OperGet() != GT_RELOAD && gtGetOp1()->OperGet() != GT_COPY);
+        assert(!gtGetOp1()->OperIs(GT_RELOAD, GT_COPY));
         return gtGetOp1();
     }
     return this;

src/coreclr/jit/stacklevelsetter.cpp

@@ -80,6 +80,14 @@ PhaseStatus StackLevelSetter::DoPhase()
             madeChanges = true;
         }
     }
+    else
+    {
+        // Mark all the throw helpers as used to avoid asserts later.
+        for (Compiler::AddCodeDsc* add = comp->fgGetAdditionalCodeDescriptors(); add != nullptr; add = add->acdNext)
+        {
+            add->acdUsed = true;
+        }
+    }
 
     // The above loop might have moved a BBJ_COND's true target to its next block.
     // In such cases, reverse the condition so we can remove a branch.
@@ -190,6 +198,7 @@ void StackLevelSetter::ProcessBlock(BasicBlock* block)
 // Arguments:
 //   node - the node to process;
 //   block - the source block for the node.
+//
 void StackLevelSetter::SetThrowHelperBlocks(GenTree* node, BasicBlock* block)
 {
     assert(node->OperMayThrow(comp));
@@ -227,11 +236,21 @@ void StackLevelSetter::SetThrowHelperBlocks(GenTree* node, BasicBlock* block)
             {
                 SetThrowHelperBlock(SCK_DIV_BY_ZERO, block);
             }
+            else
+            {
+                // Even if we thought it might divide by zero during morph, now we know it never will.
+                node->gtFlags |= GTF_DIV_MOD_NO_BY_ZERO;
+            }
 
             if ((exSetFlags & ExceptionSetFlags::ArithmeticException) != ExceptionSetFlags::None)
             {
                 SetThrowHelperBlock(SCK_ARITH_EXCPN, block);
             }
+            else
+            {
+                // Even if we thought it might overflow during morph, now we know it never will.
+                node->gtFlags |= GTF_DIV_MOD_NO_OVERFLOW;
+            }
         }
         break;
 #endif
@@ -256,10 +275,12 @@ void StackLevelSetter::SetThrowHelperBlocks(GenTree* node, BasicBlock* block)
 // Arguments:
 //   kind - the special throw-helper kind;
 //   block - the source block that targets helper.
+//
 void StackLevelSetter::SetThrowHelperBlock(SpecialCodeKind kind, BasicBlock* block)
 {
     Compiler::AddCodeDsc* add = comp->fgFindExcptnTarget(kind, comp->bbThrowIndex(block));
     assert(add != nullptr);
+
     // We expect we'll actually need this helper.
     add->acdUsed = true;