Skip to content

[Wasm RyuJIT] Fix for assert in obscure block store scenario - #126392

Merged
kg merged 5 commits into
dotnet:mainfrom
kg:wasm-blockstore-5
Apr 2, 2026
Merged

kg merged 5 commits into
dotnet:mainfrom
kg:wasm-blockstore-5

Conversation

@kg

@kg kg commented Apr 1, 2026 •

Copy link
Copy Markdown
Contributor

This fixes the assert about temporary regs from System.ValueTuple`4[System.__Canon,System.__Canon,System.__Canon,System.__Canon]:System.IComparable.CompareTo(System.Object):int:this in #125756

Copilot AI review requested due to automatic review settings April 1, 2026 01:37
@kg kg added arch-wasm WebAssembly architecture area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Apr 1, 2026
@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.

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.

Pull request overview

Fixes a WASM RyuJIT assertion/regalloc issue in an edge-case block-store lowering/codegen path by avoiding creation/consumption of temporary regs when using native memory.copy/memory.fill and the corresponding address is marked non-faulting.

Changes:

  • Adjusted WASM lowering (LowerBlockStore) to only mark src/dst addresses as multiply-used when null-checking (or non-native lowering) requires it.
  • Updated WASM codegen for GT_STORE_BLK to skip GetMultiUseOperandReg in native-op + non-faulting cases, and tightened an invariant with a debug assert.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/coreclr/jit/lowerwasm.cpp Refines when block-store src/dst addresses are marked multiply-used to avoid unnecessary temps for non-faulting native ops.
src/coreclr/jit/codegenwasm.cpp Avoids fetching temp regs for native memory.copy/fill when no null check is needed; adds an assert guarding a contained-src path.

Comment thread src/coreclr/jit/codegenwasm.cpp Outdated
Comment thread src/coreclr/jit/codegenwasm.cpp
@kg
kg marked this pull request as ready for review April 1, 2026 18:43
Copilot AI review requested due to automatic review settings April 1, 2026 18:43
@kg

kg commented Apr 1, 2026

Copy link
Copy Markdown
Contributor Author

@dotnet/jit-contrib PTAL

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread src/coreclr/jit/lowerwasm.cpp
Comment thread src/coreclr/jit/lowerwasm.cpp Outdated
Comment thread src/coreclr/jit/lowerwasm.cpp Outdated
Copilot AI review requested due to automatic review settings April 1, 2026 20:27

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@kg
kg merged commit c72e29d into dotnet:main Apr 2, 2026
139 checks passed
radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Apr 9, 2026
…126392)

This fixes the assert about temporary regs from
```System.ValueTuple`4[System.__Canon,System.__Canon,System.__Canon,System.__Canon]:System.IComparable.CompareTo(System.Object):int:this```
in dotnet#125756
@github-actions github-actions Bot locked and limited conversation to collaborators May 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasm WebAssembly architecture 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.

4 participants