[Wasm RyuJit] repair multi-entry try regions - #131558
Conversation
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 dotnet#131393 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f89c325e-8b8f-4b99-815a-89c43e95ef19
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
@adamperlin PTAL As noted in the issue the old approach was increasingly creaky. This one should be much more robust. Each dispatcher switches over the entire range of control values; while this might seem wasteful, the number of these needed (at least in SPC) is quite small: 71 methods, at most 3 dispatchers/method (average is 1.5ish), and at most 10 cases/dispatcher (average is maybe 2.5). Optimizing this to a contiguous range per switch does not yet seem worthwhile. |
There was a problem hiding this comment.
Pull request overview
Adds a new wasm-specific JIT phase to ensure all EH try regions are single-entry (entered only via ebdTryBeg), which is required to reliably express structured Wasm control flow after async/EH transforms can introduce mid-try edges.
Changes:
- Introduces
Compiler::fgWasmRepairTryEntries(newPHASE_WASM_REPAIR_TRY_ENTRIES) that rewrites side entries into try regions via cascading per-try dispatchers driven by a control local. - Removes the prior SCC-based “pseudo-successor / temporary edge” machinery used to coerce multi-entry try/catch regions into single-entry form.
- Refactors
FlowGraphTryRegionstracking to record the existence of side entries and make wasm codegen defensively NYI if any survive past the repair phase.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/jit/flowgraph.cpp | Stops special-casing async/catch resumption edges; instead records any non-header try entry as a “side entry” for wasm to reject if it remains. |
| src/coreclr/jit/fgwasm.h | Removes the wasm DFS pseudo-successor hack now that try regions are expected to be single-entry earlier. |
| src/coreclr/jit/fgwasm.cpp | Implements fgWasmRepairTryEntries; updates SCC/control-flow phases to NYI if HasSideEntry() is still true. |
| src/coreclr/jit/compphases.h | Registers the new wasm phase. |
| src/coreclr/jit/compiler.h | Renames/remodels try-region “multi-entry” tracking to m_hasSideEntry; declares fgWasmRepairTryEntries. |
| src/coreclr/jit/compiler.cpp | Wires the new phase into the wasm pipeline after wasm EH-flow + unreachable-block removal, before SCC transform. |
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
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
|
Was going to hold this until after testing is on, but am hitting this assert, so will merge this first. |
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 #131393