Skip to content

JIT: Make multiple try entries in an SCC an implLimitation in fgwasm - #134624

Merged
adamperlin merged 3 commits into
dotnet:mainfrom
adamperlin:adamperlin/wasm-multi-entry-try-impl-limit
Sep 25, 2026
Merged

adamperlin merged 3 commits into
dotnet:mainfrom
adamperlin:adamperlin/wasm-multi-entry-try-impl-limit

Conversation

@adamperlin

Copy link
Copy Markdown
Contributor

This case is very rarely hit and would be complicated to fix so it was marked as NYI before. We do hit this in real code, though not in System.Private.CoreLib. Motivated by the following CI failure when compiling Microsoft.Diagnostics.NETCore.Client

Running CrossGen2:  /root/helix/work/correlation/crossgen2/crossgen2 @/root/helix/work/workitem/e/tracing/tracing/Microsoft.Diagnostics.NETCore.Client.wasm.rsp   -r:/root/helix/work/workitem/e/tracing/tracing/IL-CG2/*.dll -f wasm
/__w/1/s/src/coreclr/jit/fgwasm.cpp:351
Assertion failed 'NYI_WASM: SCC with multiple try entry headers' in 'Microsoft.Diagnostics.NETCore.Client.ReversedDiagnosticsServer+<ListenAsync>d__18:MoveNext():this' during 'Wasm transform sccs' (IL size 888; hash 0xeb22f257; FullOpts)

@adamperlin
adamperlin requested review from AndyAyersMS and a lite review from Copilot and removed request for Copilot September 24, 2026 22:32
@github-actions github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 24, 2026
@azure-pipelines

Copy link
Copy Markdown
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.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Comment thread src/coreclr/jit/fgwasm.cpp Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Use IMPL_LIMITATION(...) for diagnostics and correct the comment grammar.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Updates WebAssembly JIT SCC handling to classify multiple try-entry headers as an implementation limitation instead of triggering an NYI assertion.

Changes:

  • Replaces the NYI path with implementation-limitation handling.
  • Documents the unsupported control-flow case.
File Description
src/​coreclr/​jit/​fgwasm.cpp Changes handling of SCCs with multiple try entries.

Comment thread src/coreclr/jit/fgwasm.cpp Outdated
Comment thread src/coreclr/jit/fgwasm.cpp Outdated
Co-authored-by: Andy Ayers <andya@microsoft.com>
Copilot AI review requested due to automatic review settings September 24, 2026 22:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Add focused regression coverage verifying unsupported methods follow the runtime-JIT-required path instead of asserting.

Review effort: Lite
Findings: 1 Low severity

Open (1)
Resolved since last review (1)

Copilot AI review requested due to automatic review settings September 24, 2026 23:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Add a regression test for the fallback path and correct the diagnostic wording.

Review effort: Lite
Findings: 1 Low severity

Open (1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants