diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index f10eb0f71a439f..55915fa329667c 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -2574,6 +2574,18 @@ void Compiler::compInitOptions(JitFlags* jitFlags) #endif // DEBUG } + bool enableInliningMethodsWithEH = JitConfig.JitInlineMethodsWithEH() > 0; + +#ifdef DEBUG + static ConfigMethodRange JitInlineMethodsWithEHRange; + JitInlineMethodsWithEHRange.EnsureInit(JitConfig.JitInlineMethodsWithEHRange()); + const unsigned hash = impInlineRoot()->info.compMethodHash(); + const bool inRange = JitInlineMethodsWithEHRange.Contains(hash); + enableInliningMethodsWithEH &= inRange; +#endif + + opts.compInlineMethodsWithEH = enableInliningMethodsWithEH; + if (compIsForInlining()) { return; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index d62ead7e3f151d..5f94d0353b6d35 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -10477,6 +10477,9 @@ class Compiler // Collect 64 bit counts for PGO data. bool compCollect64BitCounts; + // Allow inlining of methods with EH. + bool compInlineMethodsWithEH; + } opts; static bool s_pAltJitExcludeAssembliesListInitialized; diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 613a0538c71883..f6dda4d6a85764 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -3425,20 +3425,54 @@ void Compiler::fgFindBasicBlocks() unsigned XTnum; - /* Are there any exception handlers? */ - + // Are there any exception handlers? + // if (info.compXcptnsCount > 0) { - noway_assert(!compIsForInlining()); + assert(!compIsForInlining() || opts.compInlineMethodsWithEH); - /* Check and mark all the exception handlers */ + if (compIsForInlining()) + { + // Verify we can expand the EH table as needed to incorporate the callee's EH clauses. + // Failing here should be extremely rare. + // + EHblkDsc* const dsc = fgTryAddEHTableEntries(0, info.compXcptnsCount, /* deferAdding */ true); + if (dsc == nullptr) + { + compInlineResult->NoteFatal(InlineObservation::CALLSITE_EH_TABLE_FULL); + } + } + // Check and mark all the exception handlers + // for (XTnum = 0; XTnum < info.compXcptnsCount; XTnum++) { CORINFO_EH_CLAUSE clause; info.compCompHnd->getEHinfo(info.compMethodHnd, XTnum, &clause); noway_assert(clause.HandlerLength != (unsigned)-1); + // If we're inlining, and the inlinee has a catch clause, we are currently + // unable to convey the type of the catch properly, as it is represented + // by a token. So, abandon inlining. + // + // TODO: if inlining methods with catches is rare, consider + // transforming class catches into runtime filters like we do in + // fgCreateFiltersForGenericExceptions + // + if (compIsForInlining()) + { + const bool isFinallyFaultOrFilter = + (clause.Flags & (CORINFO_EH_CLAUSE_FINALLY | CORINFO_EH_CLAUSE_FAULT | CORINFO_EH_CLAUSE_FILTER)) != + 0; + + if (!isFinallyFaultOrFilter) + { + JITDUMP("Inlinee EH clause %u is a catch; we can't inline these (yet)\n", XTnum); + compInlineResult->NoteFatal(InlineObservation::CALLEE_HAS_EH); + return; + } + } + if (clause.TryLength <= 0) { BADCODE("try block length <=0"); @@ -3577,8 +3611,6 @@ void Compiler::fgFindBasicBlocks() lvaInlineeReturnSpillTempFreshlyCreated = true; } } - - return; } /* Mark all blocks within 'try' blocks as such */ diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 3d1e34a5aa1a19..c571bb92d17470 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -1035,6 +1035,19 @@ void Compiler::fgMorphCallInlineHelper(GenTreeCall* call, InlineResult* result, // Don't expect any surprises here. assert(result->IsCandidate()); +#if defined(DEBUG) + // Fail if we're inlining and we've reached the acceptance limit. + // + int limit = JitConfig.JitInlineLimit(); + unsigned current = m_inlineStrategy->GetInlineCount(); + + if ((limit >= 0) && (current >= static_cast(limit))) + { + result->NoteFatal(InlineObservation::CALLSITE_OVER_INLINE_LIMIT); + return; + } +#endif // defined(DEBUG) + if (lvaCount >= MAX_LV_NUM_COUNT_FOR_INLINING) { // For now, attributing this to call site, though it's really @@ -1568,13 +1581,179 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo) // bottomBlock->RemoveFlags(BBF_DONT_REMOVE); + // If the inlinee has EH, merge the EH tables, and figure out how much of + // a shift we need to make in the inlinee blocks EH indicies. + // + unsigned const inlineeRegionCount = InlineeCompiler->compHndBBtabCount; + const bool inlineeHasEH = inlineeRegionCount > 0; + unsigned inlineeIndexShift = 0; + + if (inlineeHasEH) + { + // If the call site also has EH, we need to insert the inlinee clauses + // so they are a child of the call site's innermost enclosing region. + // Figure out what this is. + // + bool inTryRegion = false; + unsigned const enclosingRegion = ehGetMostNestedRegionIndex(iciBlock, &inTryRegion); + + // We will insert the inlinee clauses in bulk before this index. + // + unsigned insertBeforeIndex = 0; + + if (enclosingRegion == 0) + { + // The call site is not in an EH region, so we can put the inlinee EH clauses + // at the end of root method's the EH table. + // + // For example, if the root method already has EH#0, and the inlinee has 2 regions + // + // enclosingRegion will be 0 + // inlineeIndexShift will be 1 + // insertBeforeIndex will be 1 + // + // inlinee eh0 -> eh1 + // inlinee eh1 -> eh2 + // + // root eh0 -> eh0 + // + inlineeIndexShift = compHndBBtabCount; + insertBeforeIndex = compHndBBtabCount; + } + else + { + // The call site is in an EH region, so we can put the inlinee EH clauses + // just before the enclosing region + // + // Note enclosingRegion is region index + 1. So EH#0 will be represented by 1 here. + // + // For example, if the enclosing EH regions are try#2 and hnd#3, and the inlinee has 2 eh clauses + // + // enclosingRegion will be 3 (try2 + 1) + // inlineeIndexShift will be 2 + // insertBeforeIndex will be 2 + // + // inlinee eh0 -> eh2 + // inlinee eh1 -> eh3 + // + // root eh0 -> eh0 + // root eh1 -> eh1 + // + // root eh2 -> eh4 + // root eh3 -> eh5 + // + inlineeIndexShift = enclosingRegion - 1; + insertBeforeIndex = enclosingRegion - 1; + } + + JITDUMP( + "Inlinee has EH. In root method, inlinee's %u EH region indices will shift by %u and become EH#%02u ... EH#%02u (%p)\n", + inlineeRegionCount, inlineeIndexShift, insertBeforeIndex, insertBeforeIndex + inlineeRegionCount - 1, + &inlineeIndexShift); + + if (enclosingRegion != 0) + { + JITDUMP("Inlinee is nested within current %s EH#%02u (which will become EH#%02u)\n", + inTryRegion ? "try" : "hnd", enclosingRegion - 1, enclosingRegion - 1 + inlineeRegionCount); + } + else + { + JITDUMP("Inlinee is not nested inside any EH region\n"); + } + + // Grow the EH table. + // + // TODO: verify earlier that this won't fail... + // + EHblkDsc* const outermostEbd = + fgTryAddEHTableEntries(insertBeforeIndex, inlineeRegionCount, /* deferAdding */ false); + assert(outermostEbd != nullptr); + + // fgTryAddEHTableEntries has adjusted the indices of all root method blocks and EH clauses + // to accommodate the new entries. No other changes to those are needed. + // + // We just need to add in and fix up the new entries from the inlinee. + // + // Fetch the new enclosing try/handler table indicies. + // + const unsigned enclosingTryIndex = + iciBlock->hasTryIndex() ? iciBlock->getTryIndex() : EHblkDsc::NO_ENCLOSING_INDEX; + const unsigned enclosingHndIndex = + iciBlock->hasHndIndex() ? iciBlock->getHndIndex() : EHblkDsc::NO_ENCLOSING_INDEX; + + // Copy over the EH table entries from inlinee->root and adjust their enclosing indicies. + // + for (unsigned XTnum = 0; XTnum < inlineeRegionCount; XTnum++) + { + unsigned newXTnum = XTnum + inlineeIndexShift; + compHndBBtab[newXTnum] = InlineeCompiler->compHndBBtab[XTnum]; + EHblkDsc* const ebd = &compHndBBtab[newXTnum]; + + if (ebd->ebdEnclosingTryIndex != EHblkDsc::NO_ENCLOSING_INDEX) + { + ebd->ebdEnclosingTryIndex += (unsigned short)inlineeIndexShift; + } + else + { + ebd->ebdEnclosingTryIndex = (unsigned short)enclosingTryIndex; + } + + if (ebd->ebdEnclosingHndIndex != EHblkDsc::NO_ENCLOSING_INDEX) + { + ebd->ebdEnclosingHndIndex += (unsigned short)inlineeIndexShift; + } + else + { + ebd->ebdEnclosingHndIndex = (unsigned short)enclosingHndIndex; + } + } + } + + // Fetch the new enclosing try/handler indicies for blocks. + // Note these are represented differently than the EH table indices. + // + const unsigned blockEnclosingTryIndex = iciBlock->hasTryIndex() ? iciBlock->getTryIndex() + 1 : 0; + const unsigned blockEnclosingHndIndex = iciBlock->hasHndIndex() ? iciBlock->getHndIndex() + 1 : 0; + // Set the try and handler index and fix the jump types of inlinee's blocks. // for (BasicBlock* const block : InlineeCompiler->Blocks()) { - noway_assert(!block->hasTryIndex()); - noway_assert(!block->hasHndIndex()); - block->copyEHRegion(iciBlock); + if (block->hasTryIndex()) + { + JITDUMP("Inlinee " FMT_BB " has old try index %u, shift %u, new try index %u\n", block->bbNum, + (unsigned)block->bbTryIndex, inlineeIndexShift, + (unsigned)(block->bbTryIndex + inlineeIndexShift)); + block->bbTryIndex += (unsigned short)inlineeIndexShift; + } + else + { + block->bbTryIndex = (unsigned short)blockEnclosingTryIndex; + } + + if (block->hasHndIndex()) + { + block->bbHndIndex += (unsigned short)inlineeIndexShift; + } + else + { + block->bbHndIndex = (unsigned short)blockEnclosingHndIndex; + } + + // Sanity checks + // + if (iciBlock->hasTryIndex()) + { + assert(block->hasTryIndex()); + assert(block->getTryIndex() <= iciBlock->getTryIndex()); + } + + if (iciBlock->hasHndIndex()) + { + assert(block->hasHndIndex()); + assert(block->getHndIndex() <= iciBlock->getHndIndex()); + } + block->CopyFlags(iciBlock, BBF_BACKWARD_JUMP | BBF_PROF_WEIGHT); // Update block nums appropriately @@ -1763,9 +1942,6 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo) // If the call site is not in a try and the callee has a throw, // we may introduce inconsistency. // - // Technically we should check if the callee has a throw not in a try, but since - // we can't inline methods with EH yet we don't see those. - // if (InlineeCompiler->fgThrowCount > 0) { JITDUMP("INLINER: may-throw inlinee\n"); diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 1e503daabb0f33..f945d0bb9c857d 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -6977,9 +6977,9 @@ void Compiler::impImportBlockCode(BasicBlock* block) case CEE_ENDFINALLY: - if (compIsForInlining()) + if (compIsForInlining() && !opts.compInlineMethodsWithEH) { - assert(!"Shouldn't have exception handlers in the inliner!"); + assert(!"Shouldn't have exception handlers in the inlinee!"); compInlineResult->NoteFatal(InlineObservation::CALLEE_HAS_ENDFINALLY); return; } @@ -7001,9 +7001,9 @@ void Compiler::impImportBlockCode(BasicBlock* block) case CEE_ENDFILTER: - if (compIsForInlining()) + if (compIsForInlining() && !opts.compInlineMethodsWithEH) { - assert(!"Shouldn't have exception handlers in the inliner!"); + assert(!"Shouldn't have exception handlers in the inlinee!"); compInlineResult->NoteFatal(InlineObservation::CALLEE_HAS_ENDFILTER); return; } @@ -7575,7 +7575,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) LEAVE: - if (compIsForInlining()) + if (compIsForInlining() && !opts.compInlineMethodsWithEH) { compInlineResult->NoteFatal(InlineObservation::CALLEE_HAS_LEAVE); return; @@ -11484,7 +11484,7 @@ inline void Compiler::impReimportMarkBlock(BasicBlock* block) void Compiler::impVerifyEHBlock(BasicBlock* block) { assert(block->hasTryIndex()); - assert(!compIsForInlining()); + assert(!compIsForInlining() || opts.compInlineMethodsWithEH); unsigned tryIndex = block->getTryIndex(); EHblkDsc* HBtab = ehGetDsc(tryIndex); @@ -12555,9 +12555,8 @@ void Compiler::impImport() // If the method had EH, we may be missing some pred edges // (notably those from BBJ_EHFINALLYRET blocks). Add them. - // Only needed for the root method, since inlinees can't have EH. // - if (!compIsForInlining() && (info.compXcptnsCount > 0)) + if (info.compXcptnsCount > 0) { impFixPredLists(); JITDUMP("\nAfter impImport() added blocks for try,catch,finally"); @@ -12965,7 +12964,7 @@ void Compiler::impMakeDiscretionaryInlineObservations(InlineInfo* pInlineInfo, I // // Arguments: // fncHandle -- inline candidate method -// methInfo -- method info from VN +// methInfo -- method info from VM // forceInline -- true if method is marked with AggressiveInlining // inlineResult -- ongoing inline evaluation // @@ -12979,10 +12978,13 @@ void Compiler::impCanInlineIL(CORINFO_METHOD_HANDLE fncHandle, // We shouldn't have made up our minds yet... assert(!inlineResult->IsDecided()); - if (methInfo->EHcount) + if (methInfo->EHcount > 0) { - inlineResult->NoteFatal(InlineObservation::CALLEE_HAS_EH); - return; + if (!opts.compInlineMethodsWithEH) + { + inlineResult->NoteFatal(InlineObservation::CALLEE_HAS_EH); + return; + } } if ((methInfo->ILCode == nullptr) || (codeSize == 0)) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 23ceed3cde357d..2eaff5df02f5a8 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -7763,6 +7763,25 @@ void Compiler::impMarkInlineCandidateHelper(GenTreeCall* call, return; } + if (inlineCandidateInfo->methInfo.EHcount > 0) + { + // We cannot inline methods with EH into filter clauses, even if marked as aggressive inline + // + if (bbInFilterBBRange(compCurBB)) + { + inlineResult->NoteFatal(InlineObservation::CALLSITE_IS_WITHIN_FILTER); + return; + } + + // Do not inline pinvoke stubs with EH. + // + if ((methAttr & CORINFO_FLG_PINVOKE) != 0) + { + inlineResult->NoteFatal(InlineObservation::CALLEE_HAS_EH); + return; + } + } + // The old value should be null OR this call should be a guarded devirtualization candidate. assert(call->IsGuardedDevirtualizationCandidate() || (call->GetSingleInlineCandidateInfo() == nullptr)); diff --git a/src/coreclr/jit/inline.def b/src/coreclr/jit/inline.def index 2b045ad5d20009..44d6e83929e0ba 100644 --- a/src/coreclr/jit/inline.def +++ b/src/coreclr/jit/inline.def @@ -133,6 +133,7 @@ INLINE_OBSERVATION(CANT_CLASS_INIT, bool, "can't class init", INLINE_OBSERVATION(COMPILATION_ERROR, bool, "compilation error", FATAL, CALLSITE) INLINE_OBSERVATION(COMPILATION_FAILURE, bool, "failed to compile", FATAL, CALLSITE) INLINE_OBSERVATION(EXPLICIT_TAIL_PREFIX, bool, "explicit tail prefix", FATAL, CALLSITE) +INLINE_OBSERVATION(EH_TABLE_FULL, bool, "callee has eh, eh table is full", FATAL, CALLSITE) INLINE_OBSERVATION(GENERIC_DICTIONARY_LOOKUP, bool, "runtime dictionary lookup", FATAL, CALLSITE) INLINE_OBSERVATION(HAS_CALL_VIA_LDVIRTFTN, bool, "call via ldvirtftn", FATAL, CALLSITE) INLINE_OBSERVATION(HAS_COMPLEX_HANDLE, bool, "complex handle access", FATAL, CALLSITE) diff --git a/src/coreclr/jit/inlinepolicy.cpp b/src/coreclr/jit/inlinepolicy.cpp index d22676a62a3ea2..357cf6090b7957 100644 --- a/src/coreclr/jit/inlinepolicy.cpp +++ b/src/coreclr/jit/inlinepolicy.cpp @@ -911,21 +911,6 @@ int DefaultPolicy::DetermineCallsiteNativeSizeEstimate(CORINFO_METHOD_INFO* meth void DefaultPolicy::DetermineProfitability(CORINFO_METHOD_INFO* methodInfo) { - -#if defined(DEBUG) - - // Punt if we're inlining and we've reached the acceptance limit. - int limit = JitConfig.JitInlineLimit(); - unsigned current = m_RootCompiler->m_inlineStrategy->GetInlineCount(); - - if (!m_IsPrejitRoot && (limit >= 0) && (current >= static_cast(limit))) - { - SetFailure(InlineObservation::CALLSITE_OVER_INLINE_LIMIT); - return; - } - -#endif // defined(DEBUG) - assert(InlDecisionIsCandidate(m_Decision)); assert(m_Observation == InlineObservation::CALLEE_IS_DISCRETIONARY_INLINE); @@ -1134,20 +1119,6 @@ void RandomPolicy::DetermineProfitability(CORINFO_METHOD_INFO* methodInfo) assert(InlDecisionIsCandidate(m_Decision)); assert(m_Observation == InlineObservation::CALLEE_IS_DISCRETIONARY_INLINE); -#if defined(DEBUG) - - // Punt if we're inlining and we've reached the acceptance limit. - int limit = JitConfig.JitInlineLimit(); - unsigned current = m_RootCompiler->m_inlineStrategy->GetInlineCount(); - - if (!m_IsPrejitRoot && (limit >= 0) && (current >= static_cast(limit))) - { - SetFailure(InlineObservation::CALLSITE_OVER_INLINE_LIMIT); - return; - } - -#endif // defined(DEBUG) - // Budget check. const bool overBudget = this->BudgetCheck(); if (overBudget) @@ -2400,21 +2371,6 @@ bool DiscretionaryPolicy::PropagateNeverToRuntime() const void DiscretionaryPolicy::DetermineProfitability(CORINFO_METHOD_INFO* methodInfo) { - -#if defined(DEBUG) - - // Punt if we're inlining and we've reached the acceptance limit. - int limit = JitConfig.JitInlineLimit(); - unsigned current = m_RootCompiler->m_inlineStrategy->GetInlineCount(); - - if (!m_IsPrejitRoot && (limit >= 0) && (current >= static_cast(limit))) - { - SetFailure(InlineObservation::CALLSITE_OVER_INLINE_LIMIT); - return; - } - -#endif // defined(DEBUG) - // Make additional observations based on the method info MethodInfoObservations(methodInfo); diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 82e33b64c1c051..b755323df1e466 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -119,6 +119,9 @@ CONFIG_INTEGER(JitInlinePrintStats, "JitInlinePrintStats", 0) CONFIG_INTEGER(JitInlineSize, "JITInlineSize", DEFAULT_MAX_INLINE_SIZE) CONFIG_INTEGER(JitInlineDepth, "JITInlineDepth", DEFAULT_MAX_INLINE_DEPTH) CONFIG_INTEGER(JitForceInlineDepth, "JITForceInlineDepth", DEFAULT_MAX_FORCE_INLINE_DEPTH) +RELEASE_CONFIG_INTEGER(JitInlineMethodsWithEH, "JitInlineMethodsWithEH", 1) +CONFIG_STRING(JitInlineMethodsWithEHRange, "JitInlineMethodsWithEHRange") + CONFIG_INTEGER(JitLongAddress, "JitLongAddress", 0) // Force using the large pseudo instruction form for long address CONFIG_INTEGER(JitMaxUncheckedOffset, "JitMaxUncheckedOffset", 8) diff --git a/src/coreclr/jit/jiteh.cpp b/src/coreclr/jit/jiteh.cpp index 27a3242c598508..5290a29ca24a34 100644 --- a/src/coreclr/jit/jiteh.cpp +++ b/src/coreclr/jit/jiteh.cpp @@ -1752,8 +1752,9 @@ EHblkDsc* Compiler::fgTryAddEHTableEntries(unsigned XTnum, unsigned count, bool if (deferAdding) { // We can add count entries... + // (we may not have allocated a table, so return a dummy non-null entry) // - return compHndBBtab; + return (EHblkDsc*)(0x1); } if (newCount > compHndBBtabAllocCount) @@ -3232,12 +3233,6 @@ void Compiler::dispOutgoingEHClause(unsigned num, const CORINFO_EH_CLAUSE& claus void Compiler::fgVerifyHandlerTab() { - if (compIsForInlining()) - { - // We don't inline functions with EH. Don't bother verifying the EH table in the inlinee Compiler. - return; - } - if (compHndBBtabCount == 0) { return; diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 5d85d98fc4acbc..066b8ad7347ff9 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -12466,11 +12466,16 @@ void CEEJitInfo::getEHinfo( { GetMethod(ftn)->AsDynamicMethodDesc()->GetResolver()->GetEHInfo(EHnumber, clause); } - else + else if (ftn == CORINFO_METHOD_HANDLE(m_pMethodBeingCompiled)) { - _ASSERTE(ftn == CORINFO_METHOD_HANDLE(m_pMethodBeingCompiled)); // For now only support if the method being jitted getEHinfoHelper(ftn, EHnumber, clause, m_ILHeader); } + else + { + MethodDesc* method = GetMethod(ftn); + COR_ILMETHOD_DECODER header(method->GetILHeader(), method->GetMDImport(), NULL); + getEHinfoHelper(ftn, EHnumber, clause, &header); + } EE_TO_JIT_TRANSITION(); } diff --git a/src/tests/JIT/opt/Cloning/loops_with_eh.cs b/src/tests/JIT/opt/Cloning/loops_with_eh.cs index ac0d1ee336b415..7cebcf9f7e4a7e 100644 --- a/src/tests/JIT/opt/Cloning/loops_with_eh.cs +++ b/src/tests/JIT/opt/Cloning/loops_with_eh.cs @@ -1242,7 +1242,6 @@ public static int Sum_TFLTFiTF(int[] data, int n) return sum; } - // [Fact] public static int Test_TFLITFiTF() => Sum_TFLITFiTF(data, n) - 131; public static int Sum_TFLITFiTF(int[] data, int n)