diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 041aa67e3f5c99..5ebd0dd3a83a59 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -2542,14 +2542,6 @@ void CodeGen::genReportEH() if (m_compiler->opts.dspEHTable) { printf("*************** EH table for %s\n", m_compiler->info.compFullName); - } -#endif // DEBUG - - unsigned XTnum; - -#ifdef DEBUG - if (m_compiler->opts.dspEHTable) - { printf("%d EH table entries\n", m_compiler->compHndBBtabCount); } #endif // DEBUG @@ -2561,10 +2553,11 @@ void CodeGen::genReportEH() EHClauseInfo* clauses = new (m_compiler, CMK_Codegen) EHClauseInfo[m_compiler->compHndBBtabCount]; // Set up EH clause table, but don't report anything to the VM, yet. - XTnum = 0; - for (EHblkDsc* const HBtab : EHClauses(m_compiler)) + // + for (unsigned int XTnum = 0; XTnum < m_compiler->compHndBBtabCount; XTnum++) { - UNATIVE_OFFSET tryBeg, tryEnd, hndBeg, hndEnd, hndTyp; + EHblkDsc* const HBtab = &m_compiler->compHndBBtab[XTnum]; + UNATIVE_OFFSET tryBeg, tryEnd, hndBeg, hndEnd, hndTyp; tryBeg = m_compiler->ehCodeOffset(HBtab->ebdTryBeg); hndBeg = m_compiler->ehCodeOffset(HBtab->ebdHndBeg); @@ -2593,7 +2586,10 @@ void CodeGen::genReportEH() clause.TryLength = tryEnd; clause.HandlerOffset = hndBeg; clause.HandlerLength = hndEnd; - clauses[XTnum++] = {clause, HBtab}; + + unsigned const vmIndex = m_compiler->compEHTabOrderToVMClauseOrder[XTnum]; + + clauses[vmIndex] = {clause, HBtab}; } genReportEHClauses(clauses); @@ -2605,45 +2601,43 @@ void CodeGen::genReportEH() // genReportEH: create and report EH info to the VM // // Arguments: -// clauses -- eh clause data to report +// clauses -- eh clause data to fill in and report // void CodeGen::genReportEHClauses(EHClauseInfo* clauses) { - // The JIT's ordering of EH clauses does not guarantee that clauses covering the same try region are contiguous. - // We need this property to hold true so the CORINFO_EH_CLAUSE_SAMETRY flag is accurate. - jitstd::sort(clauses, clauses + m_compiler->compHndBBtabCount, - [this](const EHClauseInfo& left, const EHClauseInfo& right) { - const unsigned short leftTryIndex = left.HBtab->ebdTryBeg->bbTryIndex; - const unsigned short rightTryIndex = right.HBtab->ebdTryBeg->bbTryIndex; + // Now, report EH clauses to the VM + // + INDEBUG(unsigned lastFuncletIndex = 0); + + for (unsigned vmIndex = 0; vmIndex < m_compiler->compHndBBtabCount; vmIndex++) + { + CORINFO_EH_CLAUSE& clause = clauses[vmIndex].clause; + unsigned const XTnum = m_compiler->compVMClauseOrderToEHTabOrder[vmIndex]; + EHblkDsc* const HBtab = &m_compiler->compHndBBtab[XTnum]; - if (leftTryIndex == rightTryIndex) +#ifdef DEBUG + // We should be seeing funclet numbers increase predictably + // as we go through the EH table in VM order. + // + if (HBtab->HasFilter()) { - // We have two clauses mapped to the same try region. - // Make sure we report the clause with the smaller index first. - const ptrdiff_t leftIndex = left.HBtab - this->m_compiler->compHndBBtab; - const ptrdiff_t rightIndex = right.HBtab - this->m_compiler->compHndBBtab; - return leftIndex < rightIndex; + assert(HBtab->ebdFuncIndex == (lastFuncletIndex + 2)); } + else + { + assert(HBtab->ebdFuncIndex == (lastFuncletIndex + 1)); + } + lastFuncletIndex = HBtab->ebdFuncIndex; +#endif - return leftTryIndex < rightTryIndex; - }); - - unsigned XTnum; - - // Now, report EH clauses to the VM in order of increasing try region index. - for (XTnum = 0; XTnum < m_compiler->compHndBBtabCount; XTnum++) - { - CORINFO_EH_CLAUSE& clause = clauses[XTnum].clause; - EHblkDsc* const HBtab = clauses[XTnum].HBtab; - - if (XTnum > 0) + if (vmIndex > 0) { // CORINFO_EH_CLAUSE_SAMETRY flag means that the current clause covers same // try block as the previous one. The runtime cannot reliably infer this information from // native code offsets because of different try blocks can have same offsets. Alternative // solution to this problem would be inserting extra nops to ensure that different try // blocks have different offsets. - if (EHblkDsc::ebdIsSameTry(HBtab, clauses[XTnum - 1].HBtab)) + if (EHblkDsc::ebdIsSameTry(HBtab, clauses[vmIndex - 1].HBtab)) { // The SAMETRY bit should only be set on catch clauses. This is ensured in IL, where only 'catch' is // allowed to be mutually-protect. E.g., the C# "try {} catch {} catch {} finally {}" actually exists in @@ -2653,10 +2647,8 @@ void CodeGen::genReportEHClauses(EHClauseInfo* clauses) } } - m_compiler->eeSetEHinfo(XTnum, &clause); + m_compiler->eeSetEHinfo(vmIndex, &clause); } - - assert(XTnum == m_compiler->compHndBBtabCount); } #ifndef TARGET_WASM diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 35be79978cc0e3..4ac75cf81de1ab 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -9459,9 +9459,11 @@ class Compiler } // Things that MAY belong either in CodeGen or CodeGenContext - FuncInfoDsc* compFuncInfos; - unsigned short compCurrFuncIdx; - unsigned short compFuncInfoCount; + FuncInfoDsc* compFuncInfos; + unsigned short compCurrFuncIdx; + unsigned short compFuncInfoCount; + unsigned short* compVMClauseOrderToEHTabOrder; + unsigned short* compEHTabOrderToVMClauseOrder; unsigned short compFuncCount() { diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 1f3def222fff24..b208a2ab990c96 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -5380,10 +5380,6 @@ void Compiler::fgMoveBlocksAfter(BasicBlock* bStart, BasicBlock* bEnd, BasicBloc // Return Value: // The last block that was relocated, or nullptr on failure. // -// Notes: -// This function can invalidate all pointers into the EH table, as well as -// change the size of the EH table! -// BasicBlock* Compiler::fgRelocateEHRange(unsigned regionIndex, FG_RELOCATE_TYPE relocateType) { INDEBUG(const char* reason = "None";) diff --git a/src/coreclr/jit/fgwasm.cpp b/src/coreclr/jit/fgwasm.cpp index 3bca9c2052ac55..66fb058fc935f3 100644 --- a/src/coreclr/jit/fgwasm.cpp +++ b/src/coreclr/jit/fgwasm.cpp @@ -2478,8 +2478,11 @@ PhaseStatus Compiler::fgWasmVirtualIP() unsigned virtualIP = 0; unsigned updatesAdded = 0; - // Prefill the EH data fields that are not dependent on - // the Virtual IP. + // Prefill the EH data fields that are not dependent on the Virtual IP. + // + // Note this and subsequent accesses to the clause info must respect + // the order in which clauses will be reported to the VM, not the order + // in the compHndBBtab. // EHClauseInfo* clauses = nullptr; @@ -2487,9 +2490,10 @@ PhaseStatus Compiler::fgWasmVirtualIP() { clauses = new (this, CMK_WasmEH) EHClauseInfo[compHndBBtabCount]; - for (EHblkDsc* const dsc : EHClauses(this)) + for (unsigned XTnum = 0; XTnum < compHndBBtabCount; XTnum++) { - const unsigned index = ehGetIndex(dsc); + EHblkDsc* const dsc = ehGetDsc(XTnum); + CORINFO_EH_CLAUSE clause; clause.ClassToken = dsc->HasFilter() ? 0 : dsc->ebdTyp; clause.Flags = ToCORINFO_EH_CLAUSE_FLAGS(dsc->ebdHandlerType); @@ -2497,7 +2501,10 @@ PhaseStatus Compiler::fgWasmVirtualIP() clause.TryLength = 0; clause.HandlerOffset = 0; clause.HandlerLength = 0; - clauses[index] = {clause, dsc}; + + unsigned const vmIndex = compEHTabOrderToVMClauseOrder[XTnum]; + + clauses[vmIndex] = {clause, dsc}; } } @@ -2588,7 +2595,10 @@ PhaseStatus Compiler::fgWasmVirtualIP() if ((tryDsc != nullptr) && (block == tryDsc->ebdTryBeg)) { virtualIP++; - clauses[block->getTryIndex()].clause.TryOffset = virtualIP; + + const unsigned tryIndex = block->getTryIndex(); + const unsigned clauseIndex = compEHTabOrderToVMClauseOrder[tryIndex]; + clauses[clauseIndex].clause.TryOffset = virtualIP; // Multiple try regions can begin at the same block. // Update all of their offsets here. @@ -2600,8 +2610,9 @@ PhaseStatus Compiler::fgWasmVirtualIP() // These should be mutual-protect trys. // assert(EHblkDsc::ebdIsSameTry(tryDsc, enclosingDsc)); - const unsigned enclosingIndex = ehGetIndex(enclosingDsc); - clauses[enclosingIndex].clause.TryOffset = virtualIP; + const unsigned enclosingTryIndex = ehGetIndex(enclosingDsc); + const unsigned enclosingClauseIndex = compEHTabOrderToVMClauseOrder[enclosingTryIndex]; + clauses[enclosingClauseIndex].clause.TryOffset = virtualIP; } } } @@ -2609,7 +2620,9 @@ PhaseStatus Compiler::fgWasmVirtualIP() if ((hndDsc != nullptr) && (block == hndDsc->ebdHndBeg)) { virtualIP++; - clauses[block->getHndIndex()].clause.HandlerOffset = virtualIP; + const unsigned hndIndex = block->getHndIndex(); + const unsigned clauseIndex = compEHTabOrderToVMClauseOrder[hndIndex]; + clauses[clauseIndex].clause.HandlerOffset = virtualIP; } if ((hndDsc != nullptr) && hndDsc->HasFilter() && (block == hndDsc->ebdFilter)) @@ -2617,7 +2630,9 @@ PhaseStatus Compiler::fgWasmVirtualIP() virtualIP++; // For filters we store the offset in the class token field. // - clauses[block->getHndIndex()].clause.ClassToken = virtualIP; + const unsigned filterIndex = block->getHndIndex(); + const unsigned clauseIndex = compEHTabOrderToVMClauseOrder[filterIndex]; + clauses[clauseIndex].clause.ClassToken = virtualIP; } // For now, just refresh the stack Virtual IP at the start of each non-empty @@ -2635,8 +2650,10 @@ PhaseStatus Compiler::fgWasmVirtualIP() if ((tryDsc != nullptr) && (block == tryDsc->ebdTryLast)) { virtualIP++; - assert(virtualIP > clauses[block->getTryIndex()].clause.TryOffset); - clauses[block->getTryIndex()].clause.TryLength = virtualIP; + const unsigned tryIndex = block->getTryIndex(); + const unsigned clauseIndex = compEHTabOrderToVMClauseOrder[tryIndex]; + assert(virtualIP > clauses[clauseIndex].clause.TryOffset); + clauses[clauseIndex].clause.TryLength = virtualIP; // Multiple try regions can end at the same block. // Update all of their extents here. @@ -2647,9 +2664,10 @@ PhaseStatus Compiler::fgWasmVirtualIP() { if (enclosingDsc->ebdTryLast == block) { - const unsigned enclosingIndex = ehGetIndex(enclosingDsc); - assert(virtualIP > clauses[enclosingIndex].clause.TryOffset); - clauses[enclosingIndex].clause.TryLength = virtualIP; + const unsigned enclosingTryIndex = ehGetIndex(enclosingDsc); + const unsigned enclosingClauseIndex = compEHTabOrderToVMClauseOrder[enclosingTryIndex]; + assert(virtualIP > clauses[enclosingClauseIndex].clause.TryOffset); + clauses[enclosingClauseIndex].clause.TryLength = virtualIP; } } } @@ -2657,8 +2675,10 @@ PhaseStatus Compiler::fgWasmVirtualIP() if ((hndDsc != nullptr) && (block == hndDsc->ebdHndLast)) { virtualIP++; - assert(virtualIP > clauses[block->getHndIndex()].clause.HandlerOffset); - clauses[block->getHndIndex()].clause.HandlerLength = virtualIP; + const unsigned hndIndex = block->getHndIndex(); + const unsigned clauseIndex = compEHTabOrderToVMClauseOrder[hndIndex]; + assert(virtualIP > clauses[clauseIndex].clause.HandlerOffset); + clauses[clauseIndex].clause.HandlerLength = virtualIP; } if ((hndDsc != nullptr) && hndDsc->HasFilter() && (block->Next() == hndDsc->ebdHndBeg)) @@ -2677,19 +2697,20 @@ PhaseStatus Compiler::fgWasmVirtualIP() JITDUMP("EH virtual IP ranges\n"); for (EHblkDsc* const dsc : EHClauses(this)) { - const unsigned index = ehGetIndex(dsc); + const unsigned index = ehGetIndex(dsc); + const unsigned clauseIndex = compEHTabOrderToVMClauseOrder[index]; - JITDUMP("EH#%02u: Try [%04u..%04u)", index, clauses[index].clause.TryOffset, - clauses[index].clause.TryLength); + JITDUMP("EH#%02u: Try [%04u..%04u)", index, clauses[clauseIndex].clause.TryOffset, + clauses[clauseIndex].clause.TryLength); if (dsc->HasFilter()) { - JITDUMP(" Filter [%04u..%04u)\n", clauses[index].clause.ClassToken, - clauses[index].clause.HandlerOffset); + JITDUMP(" Filter [%04u..%04u)\n", clauses[clauseIndex].clause.ClassToken, + clauses[clauseIndex].clause.HandlerOffset); } - JITDUMP(" Handler [%04u..%04u)\n", clauses[index].clause.HandlerOffset, - clauses[index].clause.HandlerLength); + JITDUMP(" Handler [%04u..%04u)\n", clauses[clauseIndex].clause.HandlerOffset, + clauses[clauseIndex].clause.HandlerLength); } } #endif // DEBUG diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index d03716102f0ee8..25c2f0b66f3315 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -7,7 +7,8 @@ #pragma hdrstop #endif -#include "lower.h" // for LowerRange() +#include "lower.h" // for LowerRange() +#include "jitstd/algorithm.h" // for sort() // Flowgraph Miscellany @@ -3136,10 +3137,6 @@ PhaseStatus Compiler::fgCreateFunclets() { assert(!fgFuncletsCreated); - fgCreateFuncletPrologBlocks(); - - unsigned XTnum; - EHblkDsc* HBtab; const unsigned int funcCnt = ehFuncletCount() + 1; if (!FitsIn(funcCnt)) @@ -3149,8 +3146,6 @@ PhaseStatus Compiler::fgCreateFunclets() FuncInfoDsc* funcInfo = new (this, CMK_BasicBlock) FuncInfoDsc[funcCnt]; - unsigned short funcIdx; - // Setup the root FuncInfoDsc and prepare to start associating // FuncInfoDsc's with their corresponding EH region memset((void*)funcInfo, 0, funcCnt * sizeof(FuncInfoDsc)); @@ -3167,43 +3162,87 @@ PhaseStatus Compiler::fgCreateFunclets() } #endif assert(funcInfo[0].funKind == FUNC_ROOT); - funcIdx = 1; - // Because we iterate from the top to the bottom of the compHndBBtab array, we are iterating - // from most nested (innermost) to least nested (outermost) EH region. It would be reasonable - // to iterate in the opposite order, but the order of funclets shouldn't matter. - // - // We move every handler region to the end of the function: each handler will become a funclet. - // - // Note that fgRelocateEHRange() can add new entries to the EH table. However, they will always - // be added *after* the current index, so our iteration here is not invalidated. - // It *can* invalidate the compHndBBtab pointer itself, though, if it gets reallocated! + unsigned short* vmClauseOrderToEHTabOrder = nullptr; + unsigned short* ehTabOrderToVMClauseOrder = nullptr; + unsigned short funcIdx = 1; - for (XTnum = 0; XTnum < compHndBBtabCount; XTnum++) + if (compHndBBtabCount > 0) { - HBtab = ehGetDsc(XTnum); // must re-compute this every loop, since fgRelocateEHRange changes the table - if (HBtab->HasFilter()) + fgCreateFuncletPrologBlocks(); + + // We want to have the funclet order match the order of emission of EH clauses to the runtime. + // We cannot change the order of entries in the JIT's EH table, as there are many places + // in the JIT that depend on the current ordering. + // + // So, build mappings from vm order to EH table order and vice versa. + // + vmClauseOrderToEHTabOrder = new (this, CMK_BasicBlock) unsigned short[compHndBBtabCount]; + ehTabOrderToVMClauseOrder = new (this, CMK_BasicBlock) unsigned short[compHndBBtabCount]; + + unsigned XTnum; + + for (XTnum = 0; XTnum < compHndBBtabCount; XTnum++) + { + vmClauseOrderToEHTabOrder[XTnum] = (unsigned short)XTnum; + } + + // The JIT's ordering of EH clauses does not guarantee that clauses covering the same try region are contiguous. + // We need this property to hold true so the CORINFO_EH_CLAUSE_SAMETRY flag is accurate. + // + jitstd::sort(vmClauseOrderToEHTabOrder, vmClauseOrderToEHTabOrder + compHndBBtabCount, + [this](const unsigned short& leftIndex, const unsigned short& rightIndex) { + const unsigned short leftTryIndex = ehGetDsc(leftIndex)->ebdTryBeg->bbTryIndex; + const unsigned short rightTryIndex = ehGetDsc(rightIndex)->ebdTryBeg->bbTryIndex; + + if (leftTryIndex == rightTryIndex) + { + // We have two clauses mapped to the same try region. + // Make sure we order the clause with the smaller index first. + return leftIndex < rightIndex; + } + + return leftTryIndex < rightTryIndex; + }); + + // We move every handler region to the end of the function: each handler will become a funclet. + // + for (unsigned orderNum = 0; orderNum < compHndBBtabCount; orderNum++) { + XTnum = vmClauseOrderToEHTabOrder[orderNum]; + EHblkDsc* const HBtab = ehGetDsc(XTnum); + if (HBtab->HasFilter()) + { + assert(funcIdx < funcCnt); + funcInfo[funcIdx].funKind = FUNC_FILTER; + funcInfo[funcIdx].funEHIndex = (unsigned short)XTnum; + funcIdx++; + } assert(funcIdx < funcCnt); - funcInfo[funcIdx].funKind = FUNC_FILTER; + funcInfo[funcIdx].funKind = FUNC_HANDLER; funcInfo[funcIdx].funEHIndex = (unsigned short)XTnum; + HBtab->ebdFuncIndex = funcIdx; funcIdx++; + fgRelocateEHRange(XTnum, FG_RELOCATE_HANDLER); + } + + // Fill in the inverse mapping. + // + for (unsigned i = 0; i < compHndBBtabCount; i++) + { + ehTabOrderToVMClauseOrder[vmClauseOrderToEHTabOrder[i]] = (unsigned short)i; } - assert(funcIdx < funcCnt); - funcInfo[funcIdx].funKind = FUNC_HANDLER; - funcInfo[funcIdx].funEHIndex = (unsigned short)XTnum; - HBtab->ebdFuncIndex = funcIdx; - funcIdx++; - fgRelocateEHRange(XTnum, FG_RELOCATE_HANDLER); } // We better have populated all of them by now assert(funcIdx == funcCnt); // Publish - compCurrFuncIdx = 0; - compFuncInfos = funcInfo; - compFuncInfoCount = (unsigned short)funcCnt; + compCurrFuncIdx = 0; + compFuncInfos = funcInfo; + compFuncInfoCount = (unsigned short)funcCnt; + compVMClauseOrderToEHTabOrder = vmClauseOrderToEHTabOrder; + compEHTabOrderToVMClauseOrder = ehTabOrderToVMClauseOrder; fgFuncletsCreated = true;