diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 694b0fdc9aff1..076daad229f97 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/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; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 762a445ef7ae9..9135e01cc4c45 100644 --- a/src/coreclr/jit/compiler.h +++ b/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 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(); diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 7ca688b3ada69..3337df16c7589 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/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; diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 66c0cca5195df..4b9def04ece75 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/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; } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/stacklevelsetter.cpp b/src/coreclr/jit/stacklevelsetter.cpp index 69c396b9c9767..0c66c7178dd3e 100644 --- a/src/coreclr/jit/stacklevelsetter.cpp +++ b/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. diff --git a/src/coreclr/jit/stacklevelsetter.h b/src/coreclr/jit/stacklevelsetter.h index f43558f09769c..fa788b0431f0d 100644 --- a/src/coreclr/jit/stacklevelsetter.h +++ b/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, 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 };