Fix Wasm SCC dispatcher predecessor rewrites - #133136
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Snapshot SCC header predecessors before redirecting entry edges so newly created transfer blocks are not processed again for a later header. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: db8401cb-c388-4481-a625-34811ab8a269
|
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. |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
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. |
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
|
#133134 turns on the merged runner for the wasm r2r tests, I'll update one or the other depending on the merge sequence |
Removed AlwaysUseCrossGen2 and RequiresProcessIsolation properties from the project file.
|
@AndyAyersMS I've removed the process isolation now so it will need reapproval when you get a chance |
There was a problem hiding this comment.
🟡 Changes recommended
The regression test project change likely undermines exercising the intended browser-wasm crossgen2 path and can lead to a silent false-pass.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adjusts the CoreCLR Wasm SCC switch-dispatch transformation to avoid reprocessing predecessor edges that become newly live due to earlier header rewrites, helping preserve the intended per-entry dispatch selection. It also changes the Runtime_133120 regression test project configuration for browser-wasm.
Changes:
- Snapshot SCC entry-header predecessor blocks before any edge rewrites, and skip predecessor entries that were removed by earlier header processing.
- Remove
AlwaysUseCrossGen2from theRuntime_133120test project.
File summaries
| File | Description |
|---|---|
| src/coreclr/jit/fgwasm.cpp | Snapshots original predecessor sets for SCC entry headers and avoids re-transforming newly created predecessor edges during switch-dispatch rewriting. |
| src/tests/JIT/Regression/JitBlue/Runtime_133120/Runtime_133120.csproj | Removes the browser-only AlwaysUseCrossGen2 setting from the regression test project. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
|
/ba-g failures are prexisting and unrelated |
Fixes the remaining Wasm SCC dispatch failure exposed by #133120.
This is stacked on #133132 and should be retargeted to
mainafter that PR merges.When
TransformViaSwitchDispatchprocessed SCC entry headers one at a time, redirects made transfer blocks for an earlier header become live predecessors of a later try header. The later iteration processed those blocks again and appended another control-variable store, overwriting the original dispatch case.Snapshot the original predecessor set before any rewrites, and ignore original edges that an earlier header transformation has intentionally removed. This preserves each transfer block's selected SCC entry.
The regression test is process-isolated on browser-wasm so its
AlwaysUseCrossGen2setting is exercised rather than being lost through the merged regression runner.Validation:
./build.sh clr -c Checked -os browser -a wasmRuntime_133120Runtime_133120.sh: expected 100, actual 100MoveNextJIT dump confirms the SCC transfer blocks receive one control-variable assignment eachNote
This pull request description was generated with GitHub Copilot.