JIT: removed unneeded throw helper blocks

src/coreclr/jit/compiler.cpp

@@ -5105,14 +5105,9 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
     m_pLowering = new (this, CMK_LSRA) Lowering(this, m_pLinearScan); // PHASE_LOWERING
     m_pLowering->Run();
 
-    if (!compMacOsArm64Abi())
-    {
-        // Set stack levels; this information is necessary for x86
-        // but on other platforms it is used only in asserts.
-        // TODO: do not run it in release on other platforms, see https://github.com/dotnet/runtime/issues/42673.
-        StackLevelSetter stackLevelSetter(this);
-        stackLevelSetter.Run();
-    }
+    // Set stack levels and analyze throw helper usage.
+    StackLevelSetter stackLevelSetter(this);
+    stackLevelSetter.Run();
 
     // We can not add any new tracked variables after this point.
     lvaTrackedFixed = true;

src/coreclr/jit/compiler.h

@@ -6295,18 +6295,44 @@ class Compiler
         BasicBlock*     acdDstBlk; // block  to  which we jump
         unsigned        acdData;
         SpecialCodeKind acdKind; // what kind of a special block is this?
+        bool            acdUsed; // do we need to keep this helper block?
 #if !FEATURE_FIXED_OUT_ARGS
         bool     acdStkLvlInit; // has acdStkLvl value been already set?
         unsigned acdStkLvl;     // stack level in stack slots.
 #endif                          // !FEATURE_FIXED_OUT_ARGS
     };
 
+    struct AddCodeDscKey
+    {
+    public:
+        AddCodeDscKey(): acdKind(SCK_NONE), acdData(0) {}
+        AddCodeDscKey(SpecialCodeKind kind, unsigned data): acdKind(kind), acdData(data) {}
+
+        static bool Equals(const AddCodeDscKey& x, const AddCodeDscKey& y)
+        {
+            return (x.acdData == y.acdData) && (x.acdKind == y.acdKind);
+        }
+
+        static unsigned GetHashCode(const AddCodeDscKey& x)
+        {
+            return (x.acdData << 3) | (unsigned) x.acdKind;
+        }
+
+    private:
+        SpecialCodeKind acdKind;
+        unsigned acdData;
+    };
+
+    typedef JitHashTable<AddCodeDscKey, AddCodeDscKey, AddCodeDsc*> AddCodeDscMap;
+
+    AddCodeDscMap* fgGetAddCodeDscMap();
+
 private:
     static unsigned acdHelper(SpecialCodeKind codeKind);
 
     AddCodeDsc* fgAddCodeList;
     bool        fgRngChkThrowAdded;
-    AddCodeDsc* fgExcptnTargetCache[SCK_COUNT];
+    AddCodeDscMap* fgAddCodeDscMap;
 
     void fgAddCodeRef(BasicBlock* srcBlk, SpecialCodeKind kind);
     PhaseStatus fgCreateThrowHelperBlocks();

src/coreclr/jit/fgbasic.cpp

@@ -99,13 +99,8 @@ void Compiler::fgInit()
 
     // Initialize the logic for adding code. This is used to insert code such
     // as the code that raises an exception when an array range check fails.
-
-    fgAddCodeList = nullptr;
-
-    for (int i = 0; i < SCK_COUNT; i++)
-    {
-        fgExcptnTargetCache[i] = nullptr;
-    }
+    fgAddCodeList   = nullptr;
+    fgAddCodeDscMap = nullptr;
 
     /* Keep track of the max count of pointer arguments */
     fgPtrArgCntMax = 0;

src/coreclr/jit/flowgraph.cpp

@@ -3515,6 +3515,21 @@ const char* sckName(SpecialCodeKind codeKind)
 }
 #endif
 
+//------------------------------------------------------------------------
+// fgGetAddCodeDscMap; create or return the add code desc map
+//
+// Returns:
+//   add code desc map
+//
+Compiler::AddCodeDscMap* Compiler::fgGetAddCodeDscMap()
+{
+    if (fgAddCodeDscMap == nullptr)
+    {
+        fgAddCodeDscMap = new (getAllocator(CMK_Unknown)) AddCodeDscMap(getAllocator(CMK_Unknown));
+    }
+    return fgAddCodeDscMap;
+}
+
 //------------------------------------------------------------------------
 // fgAddCodeRef: Indicate that a particular throw helper block will
 //   be needed by the method.
@@ -3568,11 +3583,21 @@ void Compiler::fgAddCodeRef(BasicBlock* srcBlk, SpecialCodeKind kind)
     add->acdStkLvlInit = false;
 #endif // !FEATURE_FIXED_OUT_ARGS
 
+    // This gets set true in the stack level setter
+    // if there's still a need for this helper
+    add->acdUsed = false;
+
     fgAddCodeList = add;
 
     // Defer creating of the blocks until later.
     //
     add->acdDstBlk = srcBlk;
+
+    // Add to map
+    //
+    AddCodeDscMap* const map = fgGetAddCodeDscMap();
+    AddCodeDscKey        key(kind, refData);
+    map->Set(key, add);
 }
 
 //------------------------------------------------------------------------
@@ -3754,41 +3779,28 @@ PhaseStatus Compiler::fgCreateThrowHelperBlocks()
 //
 // Arguments:
 //    kind - kind of exception to throw
-//    block - block where we need to throw an exception
+//    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
 //
-// Notes:
-//   We maintain a cache of one AddCodeDsc for each kind, to make searching fast.
-//
-//   Each block in an EH region uses the same (maybe shared) block as the jump target for
-//   this exception kind.
-//
 Compiler::AddCodeDsc* Compiler::fgFindExcptnTarget(SpecialCodeKind kind, unsigned refData)
 {
     assert(fgUseThrowHelperBlocks() || (kind == SCK_FAIL_FAST));
+    AddCodeDsc*          add = nullptr;
+    AddCodeDscMap* const map = fgGetAddCodeDscMap();
+    AddCodeDscKey        key(kind, refData);
+    map->Lookup(key, &add);
 
-    if (!(fgExcptnTargetCache[kind] && // Try the cached value first
-          fgExcptnTargetCache[kind]->acdData == refData))
+    if (add == nullptr)
     {
-        // Too bad, have to search for the jump target for the exception
-
-        AddCodeDsc* add = nullptr;
-
-        for (add = fgAddCodeList; add != nullptr; add = add->acdNext)
-        {
-            if (add->acdData == refData && add->acdKind == kind)
-            {
-                break;
-            }
-        }
-
-        fgExcptnTargetCache[kind] = add;
+        // We should't be asking for these blocks late in compilation
+        // unless we know there are entries to be found.
+        assert(!fgRngChkThrowAdded);
     }
 
-    return fgExcptnTargetCache[kind];
+    return add;
 }
 
 //------------------------------------------------------------------------

src/coreclr/jit/stacklevelsetter.cpp

@@ -14,9 +14,9 @@ StackLevelSetter::StackLevelSetter(Compiler* compiler)
     , maxStackLevel(0)
     , memAllocator(compiler->getAllocator(CMK_CallArgs))
     , putArgNumSlots(memAllocator)
+    , throwHelperBlocksUsed(comp->fgUseThrowHelperBlocks() && comp->compUsesThrowHelper)
 #if !FEATURE_FIXED_OUT_ARGS
     , framePointerRequired(compiler->codeGen->isFramePointerRequired())
-    , throwHelperBlocksUsed(comp->fgUseThrowHelperBlocks() && comp->compUsesThrowHelper)
 #endif // !FEATURE_FIXED_OUT_ARGS
 {
     // The constructor reads this value to skip iterations that could set it if it is already set.
@@ -59,8 +59,29 @@ PhaseStatus StackLevelSetter::DoPhase()
     comp->fgSetPtrArgCntMax(maxStackLevel);
     CheckArgCnt();
 
-    // Might want an "other" category for things like this...
-    return PhaseStatus::MODIFIED_NOTHING;
+    // When optimizing, check if there are any unused throw helper blocks,
+    // and if so, remove them.
+    //
+    bool madeChanges = false;
+
+    if (comp->opts.OptimizationEnabled())
+    {
+        for (Compiler::AddCodeDsc* add = comp->fgGetAdditionalCodeDescriptors(); add != nullptr; add = add->acdNext)
+        {
+            if (add->acdUsed)
+            {
+                continue;
+            }
+
+            BasicBlock* const block = add->acdDstBlk;
+            JITDUMP("Throw help block " FMT_BB " is unused\n", block->bbNum);
+            block->bbFlags &= ~BBF_DONT_REMOVE;
+            comp->fgRemoveBlock(block, /* unreachable */ true);
+            madeChanges = true;
+        }
+    }
+
+    return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING;
 }
 
 //------------------------------------------------------------------------
@@ -71,6 +92,9 @@ PhaseStatus StackLevelSetter::DoPhase()
 //   Nodes in blocks are iterated in the reverse order to memorize GT_PUTARG_STK
 //   and GT_PUTARG_SPLIT stack sizes.
 //
+//   Also note which (if any) throw helper blocks might end up being used by
+//   codegen.
+//
 // Arguments:
 //   block - the block to process.
 //
@@ -89,17 +113,6 @@ void StackLevelSetter::ProcessBlock(BasicBlock* block)
             SubStackLevel(numSlots);
         }
 
-#if !FEATURE_FIXED_OUT_ARGS
-        // Set throw blocks incoming stack depth for x86.
-        if (throwHelperBlocksUsed && !framePointerRequired)
-        {
-            if (node->OperMayThrow(comp))
-            {
-                SetThrowHelperBlocks(node, block);
-            }
-        }
-#endif // !FEATURE_FIXED_OUT_ARGS
-
         if (node->IsCall())
         {
             GenTreeCall* call                = node->AsCall();
@@ -108,11 +121,41 @@ void StackLevelSetter::ProcessBlock(BasicBlock* block)
             call->gtArgs.SetStkSizeBytes(usedStackSlotsCount * TARGET_POINTER_SIZE);
 #endif // UNIX_X86_ABI
         }
+
+        if (!throwHelperBlocksUsed)
+        {
+            continue;
+        }
+
+        // When optimizing we want to know what throw helpers might be used
+        // so we can remove the ones that aren't needed.
+        //
+        // If we're not optimizing then the helper requests made in
+        // morph are likely still accurate, so we don't bother checking
+        // if helpers are indeed used.
+        //
+        bool checkForHelpers = comp->opts.OptimizationEnabled();
+
+#if !FEATURE_FIXED_OUT_ARGS
+        // Even if not optimizing, if we have a moving SP frame, a shared helper may
+        // be reached with mixed stack depths and so force this method to use
+        // a frame pointer. Once we see that the method will need a frame
+        // pointer we no longer need to check for this case.
+        //
+        checkForHelpers |= !framePointerRequired;
+#endif
+
+        if (checkForHelpers)
+        {
+            if (((node->gtFlags & GTF_EXCEPT) != 0) && node->OperMayThrow(comp))
+            {
+                SetThrowHelperBlocks(node, block);
+            }
+        }
     }
     assert(currentStackLevel == 0);
 }
 
-#if !FEATURE_FIXED_OUT_ARGS
 //------------------------------------------------------------------------
 // SetThrowHelperBlocks: Set throw helper blocks incoming stack levels targeted
 //                       from the node.
@@ -147,6 +190,25 @@ void StackLevelSetter::SetThrowHelperBlocks(GenTree* node, BasicBlock* block)
             SetThrowHelperBlock(SCK_ARITH_EXCPN, block);
             break;
 
+#if defined(TARGET_ARM64) || defined(TARGET_ARM) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
+        case GT_DIV:
+        case GT_UDIV:
+        {
+            ExceptionSetFlags exSetFlags = node->OperExceptions(comp);
+
+            if ((exSetFlags & ExceptionSetFlags::DivideByZeroException) != ExceptionSetFlags::None)
+            {
+                SetThrowHelperBlock(SCK_DIV_BY_ZERO, block);
+            }
+
+            if ((exSetFlags & ExceptionSetFlags::ArithmeticException) != ExceptionSetFlags::None)
+            {
+                SetThrowHelperBlock(SCK_ARITH_EXCPN, block);
+            }
+        }
+        break;
+#endif
+
         default: // Other opers can target throw only due to overflow.
             break;
     }
@@ -171,6 +233,11 @@ void StackLevelSetter::SetThrowHelperBlock(SpecialCodeKind kind, BasicBlock* blo
 {
     Compiler::AddCodeDsc* add = comp->fgFindExcptnTarget(kind, comp->bbThrowIndex(block));
     assert(add != nullptr);
+    // We expect we'll actually need this helper.
+    add->acdUsed = true;
+
+#if !FEATURE_FIXED_OUT_ARGS
+
     if (add->acdStkLvlInit)
     {
         // If different range checks happen at different stack levels,
@@ -217,9 +284,8 @@ void StackLevelSetter::SetThrowHelperBlock(SpecialCodeKind kind, BasicBlock* blo
 #endif // Debug
         add->acdStkLvl = currentStackLevel;
     }
-}
-
 #endif // !FEATURE_FIXED_OUT_ARGS
+}
 
 //------------------------------------------------------------------------
 // PopArgumentsFromCall: Calculate the number of stack arguments that are used by the call.

src/coreclr/jit/stacklevelsetter.h

@@ -16,10 +16,8 @@ class StackLevelSetter final : public Phase
 private:
     void ProcessBlock(BasicBlock* block);
 
-#if !FEATURE_FIXED_OUT_ARGS
     void SetThrowHelperBlocks(GenTree* node, BasicBlock* block);
     void SetThrowHelperBlock(SpecialCodeKind kind, BasicBlock* block);
-#endif // !FEATURE_FIXED_OUT_ARGS
 
     unsigned PopArgumentsFromCall(GenTreeCall* call);
     void AddStackLevel(unsigned value);
@@ -35,10 +33,10 @@ class StackLevelSetter final : public Phase
     CompAllocator memAllocator;
 
     typedef JitHashTable<GenTreePutArgStk*, JitPtrKeyFuncs<GenTreePutArgStk>, unsigned> PutArgNumSlotsMap;
-    PutArgNumSlotsMap putArgNumSlots; // The hash table keeps stack slot sizes for active GT_PUTARG_STK nodes.
+    PutArgNumSlotsMap putArgNumSlots;        // The hash table keeps stack slot sizes for active GT_PUTARG_STK nodes.
+    bool              throwHelperBlocksUsed; // Were any throw helper blocks requested for this method.
 
 #if !FEATURE_FIXED_OUT_ARGS
-    bool framePointerRequired;  // Is frame pointer required based on the analysis made by this phase.
-    bool throwHelperBlocksUsed; // Were any throw helper blocks created for this method.
-#endif                          // !FEATURE_FIXED_OUT_ARGS
+    bool framePointerRequired; // Is frame pointer required based on the analysis made by this phase.
+#endif                         // !FEATURE_FIXED_OUT_ARGS
 };