From ab24778a3e54da65b8b209a51bc6e740253053d7 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 29 Jul 2026 15:02:40 -0700 Subject: [PATCH 1/2] [Wasm RyuJit] repair multi-entry try regions Runtime-async resumption and Wasm EH flow can add edges into the middle of a try region. For try/catch at least we need to transform these back to single-entry regions to match Wasm structured control flow. Previously we were leveraging the SCC transform for this, by (effectively) adding additional edges to the control flow graph between try region entries and side entries. But this has become increasingly cumbersome over time, and also cannot handle cases where the SCC entry headers span more than one try region, since the SCC transform introduces just one dispatch block. So instead, we now transform all try regions back into single entry regions, after the async and EH transforms, via a new phase fgWasmRepairTryEntries. We do this for all try regions instead of just try/catch for simplicity and consistency during layout; the issue we're fixing was a layout problem with an isolated resumption block in a try/fault. Each try region with a side entry gets a dispatcher: the header is split so it holds nothing but a switch on a control variable, with normal entry as the default case. Side entry preds set the control variable and branch to the outermost region header they enter, and the dispatchers cascade inward until the target is reached, where a landing pad resets the control variable so a later normal entry is not misdirected by a stale control variable value. Wasm try/catch headers are already split by fgWasmEhFlow, so there the dispatcher hangs off the GT_WASM_JEXCEPT false edge. We also remove the mechanisms introduced for the SCC-based solution, including the side entry accommodations in FlowGraphTryRegions, the pseudo-successor hack in VisitWasmSuccs, and the multiple-entry region edge helpers. FlowGraphTryRegions::Build now records whether a side entry exists. If so, consumers now defensively bail out with NYI_WASM since side entries are no longer expected. Fixes https://github.com/dotnet/runtime/issues/131393 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f89c325e-8b8f-4b99-815a-89c43e95ef19 --- src/coreclr/jit/compiler.cpp | 4 + src/coreclr/jit/compiler.h | 29 +-- src/coreclr/jit/compphases.h | 1 + src/coreclr/jit/fgwasm.cpp | 475 ++++++++++++++++++++++++++++++++-- src/coreclr/jit/fgwasm.h | 54 +--- src/coreclr/jit/flowgraph.cpp | 104 +------- 6 files changed, 474 insertions(+), 193 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 38e96f3ba8f9cc..edb2cce278860e 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4998,6 +4998,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // DoPhase(this, PHASE_DFS_BLOCKS_WASM, &Compiler::fgDfsBlocksAndRemove); + // Repair any multiple-entry try regions back to single entry. + // + DoPhase(this, PHASE_WASM_REPAIR_TRY_ENTRIES, &Compiler::fgWasmRepairTryEntries); + // Transform any strongly connected components into reducible flow. // DoPhase(this, PHASE_WASM_TRANSFORM_SCCS, &Compiler::fgWasmTransformSccs); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 4474bc648f2da5..a43dac9135be99 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2361,7 +2361,6 @@ class FlowGraphTryRegion jitstd::vector m_unreachableBlocks; bool m_requiresRuntimeResumption; - bool m_hasSideEntry; FlowGraphTryRegion(EHblkDsc* ehDsc, FlowGraphTryRegions* regions); @@ -2370,11 +2369,6 @@ class FlowGraphTryRegion m_requiresRuntimeResumption = true; } - void SetHasSideEntry() - { - m_hasSideEntry = true; - } - bool IsMutualProtectWith(FlowGraphTryRegion* other) const { return EHblkDsc::ebdIsSameTry(this->m_ehDsc, other->m_ehDsc); @@ -2415,13 +2409,6 @@ class FlowGraphTryRegion return m_requiresRuntimeResumption; } - // True if control can enter the try via some block other than the header block. - // - bool HasSideEntry() const - { - return m_hasSideEntry; - } - FlowGraphTryRegion* EnclosingRegion() const; BasicBlock* GetHeaderBlock() const @@ -2450,12 +2437,12 @@ class FlowGraphTryRegions unsigned m_numRegions; unsigned m_numTryCatchRegions; bool m_tryRegionsIncludeHandlerBlocks; - bool m_hasMultipleEntryTryRegions; + bool m_hasSideEntry; BitVecTraits m_traits; - void SetHasMultipleEntryTryRegions() + void SetHasSideEntry() { - m_hasMultipleEntryTryRegions = true; + m_hasSideEntry = true; } public: @@ -2493,11 +2480,10 @@ class FlowGraphTryRegions bool TryRegionsIncludeHandlerBlocks() const { return m_tryRegionsIncludeHandlerBlocks; } - bool HasMultipleEntryTryRegions() const { return m_hasMultipleEntryTryRegions; } - - void AddMultipleEntryRegionEdges(ArrayStack& edges); - - void RemoveMultipleEntryRegionEdges(ArrayStack& edges); + // True if some try region is entered at a block other than its header. + // fgWasmRepairTryEntries should have removed all of these. + // + bool HasSideEntry() const { return m_hasSideEntry; } #ifdef DEBUG static void Dump(FlowGraphTryRegions* regions); @@ -6887,6 +6873,7 @@ class Compiler void fgWasmEhTransformTry(ArrayStack* catchRetBlocks, unsigned regionIndex, unsigned catchRetIndexLocalNum); PhaseStatus fgWasmControlFlow(); PhaseStatus fgWasmTransformSccs(); + PhaseStatus fgWasmRepairTryEntries(); PhaseStatus fgWasmVirtualIP(); PhaseStatus fgWasmSpillRefs(); #ifdef DEBUG diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index 17f52494fb8181..497608425b7d03 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -129,6 +129,7 @@ CompPhaseNameMacro(PHASE_REPAIR_PROFILE_PRE_LAYOUT, "Repair profile pre-layout" CompPhaseNameMacro(PHASE_DFS_BLOCKS_WASM, "Wasm remove unreachable blocks", false, -1, false) CompPhaseNameMacro(PHASE_WASM_EH_FLOW, "Wasm eh control flow", false, -1, false) +CompPhaseNameMacro(PHASE_WASM_REPAIR_TRY_ENTRIES, "Wasm repair try entries", false, -1, false) CompPhaseNameMacro(PHASE_WASM_TRANSFORM_SCCS, "Wasm transform sccs", false, -1, false) CompPhaseNameMacro(PHASE_WASM_CONTROL_FLOW, "Wasm control flow", false, -1, false) CompPhaseNameMacro(PHASE_WASM_SPILL_REFS, "Wasm spill refs", false, -1, false) diff --git a/src/coreclr/jit/fgwasm.cpp b/src/coreclr/jit/fgwasm.cpp index afd19800c736d4..b4133770aff59d 100644 --- a/src/coreclr/jit/fgwasm.cpp +++ b/src/coreclr/jit/fgwasm.cpp @@ -1069,6 +1069,447 @@ bool FgWasm::WasmTransformSccs(ArrayStack& sccs) return modified; } +//----------------------------------------------------------------------------- +// WasmTryEntryDispatch: describes a try region that needs an entry dispatcher +// +struct WasmTryEntryDispatch +{ + // Header block of the try region (its ebdTryBeg). + BasicBlock* m_header; + + // The dispatch block created just inside the region. + BasicBlock* m_dispatcher; + + // The block that normal (non-dispatched) entry into the region should reach. + BasicBlock* m_normalTarget; +}; + +typedef JitHashTable, unsigned> WasmBlockToIndexMap; + +//----------------------------------------------------------------------------- +// fgWasmRepairTryEntries: reshape try regions so that each is entered only via +// its ebdTryBeg. +// +// Returns: +// suitable phase status +// +// Notes: +// Runtime-async resumption and catch resumption can transfer control directly +// into the middle of a try region. Wasm has no way to express this for try-catches. +// For generality we fix this for all try regions. +// +// Each region with side entries gets a dispatch block just inside its header, +// switching on a control variable that holds a dense index identifying the ultimate +// target. A side entry edge is redirected to the header of the outermost region it +// was entering, and the cascade of dispatchers steers control inward, one region at +// a time, until it reaches the target. +// +// Normal entry into a region sets the control variable to a sentinel value +// (initialized on method entry) and so falls into the dispatcher's default case. +// +PhaseStatus Compiler::fgWasmRepairTryEntries() +{ + assert(fgNodeThreading == NodeThreading::LIR); + + // If there's no EH we cannot have EH with side entries. + // + if (compHndBBtabCount == 0) + { + return PhaseStatus::MODIFIED_NOTHING; + } + + // Find the side entry edges. + // + // For each edge into a try region block, walk the block's enclosing try regions + // from innermost outward, stopping at the first region that also contains the + // source. The last walked is the outermost one the edge enters. If the edge does + // not target that region's header, it is a side entry. + // + struct SideEntry + { + BasicBlock* m_pred; + BasicBlock* m_startHeader; + BasicBlock* m_target; + }; + + CompAllocator const allocator = getAllocator(CMK_WasmCfgLowering); + ArrayStack sideEntries(allocator); + + for (BasicBlock* const block : Blocks()) + { + if (!block->hasTryIndex()) + { + continue; + } + + // A handler nested inside a try carries the enclosing try's index, but for flow + // purposes it is outside that try (see FlowGraphTryRegions::Build). Entering such + // a block is not a try region side entry. + // + if (!BasicBlock::sameHndRegion(block, ehGetDsc(block->getTryIndex())->ebdTryBeg)) + { + continue; + } + + for (FlowEdge* const edge : block->PredEdges()) + { + BasicBlock* const pred = edge->getSourceBlock(); + + // Catchret edges do not represent real Wasm control flow; resumption + // out of a catch is modelled by the post-try dispatch block. + // + if (pred->KindIs(BBJ_EHCATCHRET)) + { + continue; + } + + unsigned outermost = EHblkDsc::NO_ENCLOSING_INDEX; + for (unsigned index = block->getTryIndex(); index != EHblkDsc::NO_ENCLOSING_INDEX; + index = ehGetDsc(index)->ebdEnclosingTryIndex) + { + if (bbInTryRegions(index, pred)) + { + break; + } + + outermost = index; + } + + if (outermost == EHblkDsc::NO_ENCLOSING_INDEX) + { + // The source is already inside the innermost region, so this edge + // does not cross any region boundary. + // + continue; + } + + BasicBlock* const startHeader = ehGetDsc(outermost)->ebdTryBeg; + + if (startHeader == block) + { + // A normal entry, at the header of the outermost region entered. + // + continue; + } + + JITDUMP("Side entry " FMT_BB " -> " FMT_BB ", entering try region headed by " FMT_BB "\n", pred->bbNum, + block->bbNum, startHeader->bbNum); + + sideEntries.Push({pred, startHeader, block}); + } + } + + if (sideEntries.Empty()) + { + JITDUMP("No try region side entries\n"); + return PhaseStatus::MODIFIED_NOTHING; + } + + // Assign a dense index to each distinct side entry target, and record the chain of + // try region headers the dispatch must traverse to reach it: innermost first, + // ending at the header of the outermost region any side entry to it enters. + // + // A target can have several side entries entering at different depths; the longest + // chain covers them all. + // + ArrayStack targets(allocator); + jitstd::vector> targetPaths(allocator); + WasmBlockToIndexMap giveIndex(allocator); + + for (SideEntry const& sideEntry : sideEntries.BottomUpOrder()) + { + jitstd::vector path(allocator); + for (unsigned index = sideEntry.m_target->getTryIndex(); index != EHblkDsc::NO_ENCLOSING_INDEX; + index = ehGetDsc(index)->ebdEnclosingTryIndex) + { + BasicBlock* const header = ehGetDsc(index)->ebdTryBeg; + path.push_back(header); + + if (header == sideEntry.m_startHeader) + { + break; + } + } + + assert(!path.empty()); + assert(path.back() == sideEntry.m_startHeader); + + unsigned index = 0; + if (giveIndex.Lookup(sideEntry.m_target, &index)) + { + // Both chains start at the target's own try region and walk outward through + // the same enclosing region sequence, just stopping at different depths. So + // the shorter must be a prefix of the longer, and keeping the longer covers + // every side entry to this target. + // +#ifdef DEBUG + jitstd::vector const& existing = targetPaths[index]; + size_t const common = min(existing.size(), path.size()); + for (size_t p = 0; p < common; p++) + { + assert(existing[p] == path[p]); + } +#endif + + if (path.size() > targetPaths[index].size()) + { + targetPaths[index] = std::move(path); + } + } + else + { + index = (unsigned)targets.Height(); + giveIndex.Set(sideEntry.m_target, index); + targets.Push(sideEntry.m_target); + targetPaths.push_back(std::move(path)); + JITDUMP("Side entry target %u is " FMT_BB "\n", index, sideEntry.m_target->bbNum); + } + } + + unsigned const numTargets = (unsigned)targets.Height(); + + // Every region on some target's cascade chain needs a dispatcher. + // + ArrayStack dispatches(allocator); + WasmBlockToIndexMap giveDispatch(allocator); + + for (unsigned v = 0; v < numTargets; v++) + { + for (BasicBlock* const header : targetPaths[v]) + { + unsigned dispatchIndex = 0; + if (!giveDispatch.Lookup(header, &dispatchIndex)) + { + giveDispatch.Set(header, (unsigned)dispatches.Height()); + dispatches.Push({header, nullptr, nullptr}); + } + } + } + + // Create the control variable and initialize it to a sentinel that lands in + // every dispatcher's default case. + // + unsigned const controlVarNum = lvaGrabTemp(/* shortLifetime */ false DEBUGARG("Wasm try entry dispatch")); + lvaGetDesc(controlVarNum)->lvType = TYP_INT; + + // fgFirstBB cannot be a dispatch header. + // + assert(!fgFirstBB->hasTryIndex() && !fgFirstBB->hasHndIndex()); + + { + GenTree* const sentinel = gtNewIconNode((ssize_t)numTargets, TYP_INT); + GenTree* const store = gtNewStoreLclVarNode(controlVarNum, sentinel); + LIR::Range range = LIR::SeqTree(this, store); + LIR::AsRange(fgFirstBB).InsertAtBeginning(std::move(range)); + } + + // Create the dispatch blocks. This must happen before we wire up any switch + // cases, since a case in one region may target the header of another. + // + for (WasmTryEntryDispatch& dispatch : dispatches.BottomUpOrder()) + { + BasicBlock* const header = dispatch.m_header; + + // For a Wasm try/catch the header has already been split so that it holds + // just the GT_WASM_JEXCEPT test, and normal entry continues on its false + // edge. Otherwise split the header so the code it holds moves out of the way. + // + GenTree* const lastNode = header->GetLastLIRNode(); + + if ((lastNode != nullptr) && lastNode->OperIs(GT_WASM_JEXCEPT)) + { + dispatch.m_normalTarget = header->GetFalseTarget(); + } + else + { + dispatch.m_normalTarget = fgSplitBlockAtBeginning(header); + } + + dispatch.m_dispatcher = fgNewBBafter(BBJ_SWITCH, header, /* extendRegion */ false); + dispatch.m_dispatcher->copyEHRegion(dispatch.m_normalTarget); + dispatch.m_dispatcher->inheritWeight(header); + + fgReplaceJumpTarget(header, dispatch.m_normalTarget, dispatch.m_dispatcher); + + JITDUMP("Try region headed by " FMT_BB ": dispatcher " FMT_BB ", normal entry " FMT_BB "\n", header->bbNum, + dispatch.m_dispatcher->bbNum, dispatch.m_normalTarget->bbNum); + } + + // Wire up each dispatcher. + // + // For control var value v, a given dispatcher is only involved if its header is on + // targets[v]'s cascade chain. At the innermost header's dispatcher control goes to + // the target itself (or, when the target is this header, to the region's normal + // entry) through a landing pad that restores the sentinel. Further out, control goes + // to the next header inward; that header's dispatcher continues the cascade. Values + // not on the chain, and the sentinel, use the default case. + // + for (WasmTryEntryDispatch const& dispatch : dispatches.BottomUpOrder()) + { + BasicBlock* const header = dispatch.m_header; + BasicBlock* const dispatcher = dispatch.m_dispatcher; + unsigned const caseCount = numTargets + 1; + + FlowEdge** const cases = new (this, CMK_FlowEdge) FlowEdge*[caseCount]; + FlowEdge** const succs = new (this, CMK_FlowEdge) FlowEdge*[caseCount]; + unsigned numUniqueSuccs = 0; + + BlockToBlockMap resetPads(allocator); + BlockToFlowEdgeMap caseEdges(allocator); + + for (unsigned v = 0; v <= numTargets; v++) + { + BasicBlock* caseTarget = dispatch.m_normalTarget; + + if (v < numTargets) + { + jitstd::vector const& path = targetPaths[v]; + + size_t pos = path.size(); + for (size_t p = 0; p < path.size(); p++) + { + if (path[p] == header) + { + pos = p; + break; + } + } + + // This dispatcher must handle this case value + // + if (pos == 0) + { + // This is the innermost dispatcher. Route flow through a per-target + // pad that resets the control var to a sentinel, so a subsequent + // normal entry to this region is not misdirected by a stale value. + // + BasicBlock* const dest = + (targets.BottomRef((int)v) == header) ? dispatch.m_normalTarget : targets.BottomRef((int)v); + + if (!resetPads.Lookup(dest, &caseTarget)) + { + BasicBlock* const pad = fgNewBBafter(BBJ_ALWAYS, dispatcher, /* extendRegion */ false); + pad->copyEHRegion(dest); + pad->inheritWeightPercentage(dispatcher, 0); + + FlowEdge* const padEdge = fgAddRefPred(dest, pad); + padEdge->setLikelihood(1.0); + pad->SetTargetEdge(padEdge); + + GenTree* const sentinel = gtNewIconNode((ssize_t)numTargets, TYP_INT); + GenTree* const store = gtNewStoreLclVarNode(controlVarNum, sentinel); + LIR::Range range = LIR::SeqTree(this, store); + LIR::AsRange(pad).InsertAtEnd(std::move(range)); + + resetPads.Set(dest, pad); + caseTarget = pad; + + JITDUMP("Reset pad " FMT_BB " for " FMT_BB "\n", pad->bbNum, dest->bbNum); + } + } + else if (pos < path.size()) + { + // Cascade inward. The next header always has a dispatcher, since + // dispatchers are created for every block on every chain. + // + caseTarget = path[pos - 1]; + assert(giveDispatch.Lookup(caseTarget)); + } + } + + FlowEdge* caseEdge = nullptr; + if (caseEdges.Lookup(caseTarget, &caseEdge)) + { + caseEdge->incrementDupCount(); + caseTarget->bbRefs++; + } + else + { + caseEdge = fgAddRefPred(caseTarget, dispatcher); + + // The normal entry uses the default. Assume it carries all the profile + // weight and that all side entries are cold. + // + caseEdge->setLikelihood((v == numTargets) ? 1.0 : 0.0); + caseEdges.Set(caseTarget, caseEdge); + succs[numUniqueSuccs++] = caseEdge; + } + + cases[v] = caseEdge; + } + + BBswtDesc* const swtDesc = new (this, CMK_BasicBlock) BBswtDesc(succs, numUniqueSuccs, cases, caseCount, true); + dispatcher->SetSwitch(swtDesc); + + GenTree* const controlVar = gtNewLclvNode(controlVarNum, TYP_INT); + GenTree* const switchNode = gtNewOperNode(GT_SWITCH, TYP_VOID, controlVar); + + assert(dispatcher->isEmpty()); + LIR::Range range = LIR::SeqTree(this, switchNode); + LIR::AsRange(dispatcher).InsertAtEnd(std::move(range)); + + JITDUMP("Dispatcher " FMT_BB " for region headed by " FMT_BB ": %u cases, %u unique successors\n", + dispatcher->bbNum, header->bbNum, caseCount, numUniqueSuccs); + } + + fgHasSwitch = true; + + // Finally, redirect the side entry edges. + // + for (SideEntry const& sideEntry : sideEntries.BottomUpOrder()) + { + BasicBlock* const pred = sideEntry.m_pred; + + unsigned index = 0; + bool const found = giveIndex.Lookup(sideEntry.m_target, &index); + assert(found); + + // Sink the control var store into the pred when it has no other successor; + // otherwise split the edge and put it there. + // + BasicBlock* transferBlock; + if (pred->HasTarget() && (pred->GetTarget() == sideEntry.m_target) && !pred->isBBCallFinallyPairTail()) + { + transferBlock = pred; + } + else + { + transferBlock = fgSplitEdge(pred, sideEntry.m_target); + } + + GenTree* const targetIndex = gtNewIconNode((ssize_t)index, TYP_INT); + GenTree* const store = gtNewStoreLclVarNode(controlVarNum, targetIndex); + LIR::Range range = LIR::SeqTree(this, store); + + if (transferBlock->isEmpty()) + { + LIR::AsRange(transferBlock).InsertAtEnd(std::move(range)); + } + else + { + LIR::InsertBeforeTerminator(transferBlock, std::move(range)); + } + + fgReplaceJumpTarget(transferBlock, sideEntry.m_target, targetPaths[index].back()); + + JITDUMP("Side entry to " FMT_BB " (index %u) now enters via " FMT_BB " from " FMT_BB "\n", + sideEntry.m_target->bbNum, index, targetPaths[index].back()->bbNum, transferBlock->bbNum); + } + + // Weight moved from the side entry targets to the region headers, so any profile + // data we had is no longer self-consistent. + // + if (fgPgoConsistent) + { + JITDUMP("Profile is now inconsistent: try region entry flow was rerouted\n"); + fgPgoConsistent = false; + } + + fgInvalidateDfsTree(); + + return PhaseStatus::MODIFIED_EVERYTHING; +} + //----------------------------------------------------------------------------- // fgWasmTransformSccs: transform SCCs into reducible flow // @@ -1088,18 +1529,16 @@ PhaseStatus Compiler::fgWasmTransformSccs() FlowGraphDfsTree* const dfsTree = fgWasm.WasmDfs(hasBlocksOnlyReachableViaEH); fgWasm.SetDfsAndTraits(dfsTree); - // Build the try region descriptors, and if there are any regions with multiple - // entry blocks, add temporary flow edges from those blocks to the enclosing try region - // main entry blocks, making the try regions look like loops. + // Build the try region descriptors. // - FlowGraphTryRegions* tryRegions = FlowGraphTryRegions::Build(this, dfsTree); - ArrayStack temporaryEdges(getAllocator(CMK_WasmSccTransform)); + FlowGraphTryRegions* tryRegions = FlowGraphTryRegions::Build(this, dfsTree); + JITDUMPEXEC(FlowGraphTryRegions::Dump(tryRegions)); - if (tryRegions->HasMultipleEntryTryRegions()) + // fgWasmRepairTryEntries just ran, so every region should be single entry. + // + if (tryRegions->HasSideEntry()) { - JITDUMP("\nThere are try regions with multiple entries.\n"); - JITDUMPEXEC(FlowGraphTryRegions::Dump(tryRegions)); - tryRegions->AddMultipleEntryRegionEdges(temporaryEdges); + NYI_WASM("Try region side entry survived fgWasmRepairTryEntries"); } FlowGraphNaturalLoops* const loops = FlowGraphNaturalLoops::Find(dfsTree); @@ -1115,10 +1554,6 @@ PhaseStatus Compiler::fgWasmTransformSccs() fgWasm.WasmFindSccs(sccs); assert(!sccs.Empty()); - // Remove the temporary edges before transforming the SCCs. - // - tryRegions->RemoveMultipleEntryRegionEdges(temporaryEdges); - transformed = fgWasm.WasmTransformSccs(sccs); assert(transformed); @@ -1133,11 +1568,6 @@ PhaseStatus Compiler::fgWasmTransformSccs() assert(loops2->ImproperLoopHeaders() == 0); #endif } - else - { - assert(!tryRegions->HasMultipleEntryTryRegions()); - assert(temporaryEdges.Empty()); - } return transformed ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } @@ -1239,12 +1669,13 @@ PhaseStatus Compiler::fgWasmControlFlow() FlowGraphTryRegions* const tryRegions = FlowGraphTryRegions::Build(this, dfsTree); JITDUMPEXEC(FlowGraphTryRegions::Dump(tryRegions)); - // We cannot handle multiple entry try regions yet. + // A wasm try region is a lexically nested construct, so it can only be entered at + // its header. Nothing between fgWasmRepairTryEntries and here should introduce a + // side entry, but if one appears we cannot express the region. // - if (tryRegions->HasMultipleEntryTryRegions()) + if (tryRegions->HasSideEntry()) { - JITDUMP("\nThere are multiple entry try regions\n"); - NYI_WASM("Multiple entry try regions"); + NYI_WASM("Try region side entry"); } // Our interval ends are at the starts of blocks, so we need a block that diff --git a/src/coreclr/jit/fgwasm.h b/src/coreclr/jit/fgwasm.h index 2832967bc0bfc3..ce6cc3c43ee648 100644 --- a/src/coreclr/jit/fgwasm.h +++ b/src/coreclr/jit/fgwasm.h @@ -566,59 +566,9 @@ BasicBlockVisit FgWasm::VisitWasmSuccs(Compiler* comp, BasicBlock* block, TFunc unreached(); } - // If the compiler has a multi-entry try region object, add edges from any - // catch resumption or async resumption target to the header of each enclosing - // try/catch. + // Try regions are single-entry by the time the wasm DFS runs (see + // Compiler::fgWasmRepairTryEntries), so no pseudo-successors are needed here. // - // This makes multi-entry try/catch regions look like multi-entry loops and the SCC - // algorithm will transform them into single-entry try/catch regions. - // - // Note we disregard try/finally/fault here as those do not need to be expressed - // as single-entry regions for Wasm codegen. And we consider all mutual-protect - // try/catch as a single region. - // - FlowGraphTryRegions* const tryRegions = comp->fgTryRegions; - - if ((tryRegions == nullptr) || !tryRegions->HasMultipleEntryTryRegions()) - { - return BasicBlockVisit::Continue; - } - - EHblkDsc* const dsc = comp->ehGetBlockTryDsc(block); - - if (dsc == nullptr) - { - return BasicBlockVisit::Continue; - } - - FlowGraphTryRegion* region = tryRegions->GetTryRegionByHeader(dsc->ebdTryBeg); - - // TODO: possibly flag blocks that are targets of resumption switches so - // we can quickly screen out blocks that are not try region side entries. - // - while (region != nullptr) - { - if (region->HasCatchHandler()) - { - if (!region->HasSideEntry()) - { - break; - } - - BasicBlock* const header = region->GetHeaderBlock(); - for (FlowEdge* const edge : region->EntryEdges()) - { - if ((block != header) && (block == edge->getDestinationBlock())) - { - assert(edge->getSourceBlock()->HasAnyFlag(BBF_ASYNC_RESUMPTION | BBF_CATCH_RESUMPTION)); - RETURN_ON_ABORT(func(header)); - break; - } - } - } - region = region->EnclosingRegion(); - } - return BasicBlockVisit::Continue; } diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 908aad169f4104..85bd63121d5559 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -7735,7 +7735,7 @@ FlowGraphTryRegions::FlowGraphTryRegions(Compiler* comp, FlowGraphDfsTree* dfsTr , m_numRegions(0) , m_numTryCatchRegions(0) , m_tryRegionsIncludeHandlerBlocks(false) - , m_hasMultipleEntryTryRegions(false) + , m_hasSideEntry(false) , m_traits((dfsTree == nullptr) ? comp->fgBBNumMax + 1 : dfsTree->GetPostOrderCount(), comp) { } @@ -7782,7 +7782,6 @@ FlowGraphTryRegion::FlowGraphTryRegion(EHblkDsc* ehDsc, FlowGraphTryRegions* reg , m_entryEdges(regions->GetCompiler()->getAllocator(CMK_BasicBlock)) , m_unreachableBlocks(regions->GetCompiler()->getAllocator(CMK_BasicBlock)) , m_requiresRuntimeResumption(false) - , m_hasSideEntry(false) { BitVecTraits* const traits = regions->GetBlockBitVecTraits(); m_blocks = BitVecOps::MakeEmpty(traits); @@ -7920,31 +7919,14 @@ FlowGraphTryRegions* FlowGraphTryRegions::Build(Compiler* comp, FlowGraphDfsTree continue; } - // Async resumption and catch resumption entry edges + // Any other edge is a side entry, which fgWasmRepairTryEntries + // should already have routed through the region header. Record it + // so wasm codegen can bail out rather than emit a region that + // cannot be expressed. // - if (predBlock->HasAnyFlag(BBF_ASYNC_RESUMPTION | BBF_CATCH_RESUMPTION)) - { - JITDUMP("Found %s resumption edge from " FMT_BB " to " FMT_BB "\n", - predBlock->HasFlag(BBF_ASYNC_RESUMPTION) ? "async" : "catch", predBlock->bbNum, - block->bbNum); - - region->AddEntryEdge(edge); - region->SetHasSideEntry(); - - // Only try/catch regions need to be reshaped into single-entry form for - // Wasm codegen (they will be lowered to a wasm try_table). Try/fault and - // try/finally are emitted differently and tolerate multi-entry. - // - if (dsc->HasCatchHandler()) - { - regions->SetHasMultipleEntryTryRegions(); - } - continue; - } - JITDUMP("Unexpected try region entry edge from " FMT_BB " to " FMT_BB "\n", predBlock->bbNum, block->bbNum); - assert(!"Unexpected try region entry edge"); + regions->SetHasSideEntry(); } region = region->m_parent; @@ -7975,80 +7957,6 @@ FlowGraphTryRegions* FlowGraphTryRegions::Build(Compiler* comp, FlowGraphDfsTree return regions; } -//------------------------------------------------------------------------ -// FlowGraphTryRegions::AddMultipleEntryRegionEdges: Add temporary -// edges for multiple entry try regions. -// -// Arguments: -// edges -- collection of temporary edges to augment -// -void FlowGraphTryRegions::AddMultipleEntryRegionEdges(ArrayStack& edges) -{ - for (FlowGraphTryRegion* region : m_tryRegions) - { - if (region != nullptr && region->HasCatchHandler() && region->HasSideEntry()) - { - BasicBlock* const headerBlock = region->GetHeaderBlock(); - - for (FlowEdge* edge : region->EntryEdges()) - { - BasicBlock* const destBlock = edge->getDestinationBlock(); - - // Skip the normal entry edges. - // - if (destBlock == headerBlock) - { - continue; - } - - // We need an edge from dest to try header. - FlowEdge* const destheaderEdge = m_compiler->fgAddRefPred(headerBlock, destBlock); - edges.Push(destheaderEdge); - - // And an edge from method entry to dest. - FlowEdge* const entryDestEdge = m_compiler->fgAddRefPred(destBlock, m_compiler->fgFirstBB); - edges.Push(entryDestEdge); - - // If the dest is not reachable within the try, then we need to also add - // a temporary edge from the try header to the dest to create the SCC. - // Since we've pruned away dead blocks, any other pred edge suffices to - // establish reachability. - // - bool isReachableInTry = false; - for (FlowEdge* const predEdge : destBlock->PredEdges()) - { - if (predEdge != edge) - { - isReachableInTry = true; - break; - } - } - - if (!isReachableInTry) - { - FlowEdge* const headerDestEdge = m_compiler->fgAddRefPred(destBlock, headerBlock); - edges.Push(headerDestEdge); - } - } - } - } -} - -//------------------------------------------------------------------------ -// FlowGraphTryRegions::RemoveMultipleEntryRegionEdges: Remove temporary -// edges added for multiple entry try regions. -// -// Arguments: -// edges -- collection of edges to remove -// -void FlowGraphTryRegions::RemoveMultipleEntryRegionEdges(ArrayStack& edges) -{ - for (FlowEdge* const edge : edges.BottomUpOrder()) - { - m_compiler->fgRemoveRefPred(edge); - } -} - //------------------------------------------------------------------------ // FlowGraphTryRegion::NumBlocks: Return the number of blocks in the try region. // From dabe5d6690d9382d9e2c09acf525b2b7a4af81c6 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 29 Jul 2026 17:09:10 -0700 Subject: [PATCH 2/2] Set the dispatcher default edge likelihood after wiring all cases A case value a dispatcher does not route also falls to the region's normal entry, so it shares the default's flow edge. Setting the likelihood while wiring cases left that shared edge cold, since the earlier cold case created it and the default only bumped the dup count. Assign the default likelihood once all cases are wired instead. Affects 12 of 104 dispatchers in System.Private.CoreLib; all now have outgoing likelihoods summing to 1.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f89c325e-8b8f-4b99-815a-89c43e95ef19 --- src/coreclr/jit/fgwasm.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/fgwasm.cpp b/src/coreclr/jit/fgwasm.cpp index b4133770aff59d..77b735bdc561e3 100644 --- a/src/coreclr/jit/fgwasm.cpp +++ b/src/coreclr/jit/fgwasm.cpp @@ -1426,11 +1426,7 @@ PhaseStatus Compiler::fgWasmRepairTryEntries() else { caseEdge = fgAddRefPred(caseTarget, dispatcher); - - // The normal entry uses the default. Assume it carries all the profile - // weight and that all side entries are cold. - // - caseEdge->setLikelihood((v == numTargets) ? 1.0 : 0.0); + caseEdge->setLikelihood(0.0); caseEdges.Set(caseTarget, caseEdge); succs[numUniqueSuccs++] = caseEdge; } @@ -1438,6 +1434,13 @@ PhaseStatus Compiler::fgWasmRepairTryEntries() cases[v] = caseEdge; } + // The normal entry uses the default. Assume it carries all the profile weight + // and that all side entries are cold. This must happen after the loop above, + // since a case value this dispatcher does not route also falls to the normal + // entry and so shares the default's edge. + // + cases[numTargets]->setLikelihood(1.0); + BBswtDesc* const swtDesc = new (this, CMK_BasicBlock) BBswtDesc(succs, numUniqueSuccs, cases, caseCount, true); dispatcher->SetSwitch(swtDesc);