[Wasm RyuJit] More crossgen assert fixes by AndyAyersMS · Pull Request #125102 · dotnet/runtime

AndyAyersMS · 2026-03-03T04:40:05Z

use gtOverflowEx instead of gtOverflow
not all block stores need null checks
more cases where we don't have putarg stacks
add lowering was bypassing Wasm overflow checking
tolerate nullchecks that can't throw (see JIT: TransformUnusedIndirection may create unused non-faulting loads #125203)
storeind nullchecks

* use gtOverflowEx instead of gtOverflow * not all block stores need null checks * more cases where we don't have putarg stacks * add lowering was bypassing Wasm overflow checking

AndyAyersMS · 2026-03-03T04:40:52Z

We might be able to be even more fine-grained with the null checks.

Also not 100% sure yet what to do with all the operand reordering for calls, but this at least tolerates the way Wasm does things.

Copilot

Pull request overview

This PR fixes several assert failures in the WASM RyuJIT cross-compilation codepath ("crossgen assert fixes"):

Changes:

Replace incorrect gtOverflow() calls with gtOverflowEx() in WASM-specific lowering and codegen, since gtOverflow() asserts OperMayOverflow() and these sites can be reached with non-overflow-capable operators (AND, OR, XOR).
Fix overflow checking for GT_ADD on WASM: LowerAdd was not calling LowerBinaryArithmetic, so the SetMultiplyUsed() calls needed to set up overflow-checking registers were skipped for overflow ADDs.
Guard null checks in genCodeForCpObj behind a doNullChecks flag (GTF_EXCEPT && OperMayThrow) since not all block-copy operations need null checks.
Fix asserts in MovePutArgUpToCall/MovePutArgNodesUpToCall that assumed nodes must be GT_PUTARG_* or GT_FIELD_LIST, which is not true on WASM where HAS_FIXED_REGISTER_SET = 0.

Reviewed changes

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

File	Description
`src/coreclr/jit/lowerwasm.cpp`	Use `gtOverflowEx()` instead of `gtOverflow()` in `LowerBinaryArithmetic` for correctness when called with non-overflow-capable operators (AND/OR/XOR)
`src/coreclr/jit/lower.cpp`	Fix `MovePutArgUpToCall`/`MovePutArgNodesUpToCall` asserts for WASM (no PutArg nodes); add new `else` branch for plain arg nodes; call `LowerBinaryArithmetic` from `LowerAdd` on WASM to fix missing overflow setup for ADD
`src/coreclr/jit/codegenwasm.cpp`	Use `gtOverflowEx()` instead of `gtOverflow()` in `genCodeForBinary`; gate null checks in `genCodeForCpObj` behind `doNullChecks` for correctness when copies are known safe

src/coreclr/jit/lower.cpp

src/coreclr/jit/codegenwasm.cpp

dotnet-policy-service · 2026-03-04T08:31:43Z

Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara
See info in area-owners.md if you want to be subscribed.

dotnet-policy-service · 2026-03-04T08:31:43Z

Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

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

src/coreclr/jit/regallocwasm.cpp

SingleAccretion

LGTM modulo comments.

src/coreclr/jit/regallocwasm.cpp

src/coreclr/jit/codegenwasm.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/jit/lower.cpp:8056

In LowerAdd, the LowerBinaryArithmetic(node) call (line 8049) already invokes ContainCheckBinary internally (see lowerwasm.cpp line 174). The code then falls through to call ContainCheckBinary(node) again at line 8055 for WASM. Since WASM's ContainCheckBinary is an empty no-op, this is functionally harmless but is a redundant double call. Returning early from the WASM block (or not calling ContainCheckBinary again for WASM) would be cleaner.

#if defined(TARGET_WASM)
    if (node->OperIs(GT_ADD))
    {
        LowerBinaryArithmetic(node);
    }
#endif

    if (node->OperIs(GT_ADD))
    {
        ContainCheckBinary(node);
    }

AndyAyersMS · 2026-03-04T22:40:44Z

@dotnet/jit-contrib PTAL

Copilot

Pull request overview

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

src/coreclr/jit/codegenwasm.cpp

Copilot

Pull request overview

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

[Wasm RyuJit] More crossgen assert fixes

5f11a79

* use gtOverflowEx instead of gtOverflow * not all block stores need null checks * more cases where we don't have putarg stacks * add lowering was bypassing Wasm overflow checking

Copilot AI review requested due to automatic review settings March 3, 2026 04:40

github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 3, 2026

dotnet-policy-service bot assigned AndyAyersMS Mar 3, 2026

Copilot started reviewing on behalf of AndyAyersMS March 3, 2026 04:40 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

src/coreclr/jit/lower.cpp Show resolved Hide resolved

src/coreclr/jit/lower.cpp Show resolved Hide resolved

jakobbotsch reviewed Mar 3, 2026

View reviewed changes

src/coreclr/jit/lower.cpp Show resolved Hide resolved

SingleAccretion reviewed Mar 3, 2026

View reviewed changes

src/coreclr/jit/codegenwasm.cpp Outdated Show resolved Hide resolved

AndyAyersMS added 4 commits March 3, 2026 13:33

review feedback

e2f8b7c

review feedback

006b0ff

null checks for storeind

0545afe

only null check if not a gc write barrier

1b6af29

am11 added arch-wasm WebAssembly architecture labels Mar 4, 2026

AndyAyersMS added 2 commits March 4, 2026 07:18

Merge branch 'main' into WasmFixes5

5e26e03

fix build

5c9aec7

Copilot AI review requested due to automatic review settings March 4, 2026 15:22

Copilot started reviewing on behalf of AndyAyersMS March 4, 2026 15:25 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

src/coreclr/jit/regallocwasm.cpp Outdated Show resolved Hide resolved

SingleAccretion reviewed Mar 4, 2026

View reviewed changes

src/coreclr/jit/regallocwasm.cpp Outdated Show resolved Hide resolved

src/coreclr/jit/codegenwasm.cpp Outdated Show resolved Hide resolved

Apply suggestions from code review

51757b7

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>

Copilot AI review requested due to automatic review settings March 4, 2026 15:59

Copilot started reviewing on behalf of AndyAyersMS March 4, 2026 16:00 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

null check storeinds in codegen even if they are write barriers

02fde2f

AndyAyersMS marked this pull request as ready for review March 4, 2026 22:38

Copilot AI review requested due to automatic review settings March 4, 2026 22:38

Copilot started reviewing on behalf of AndyAyersMS March 4, 2026 22:39 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

tolerate null check that can't fault

ff02b8d

kg reviewed Mar 5, 2026

View reviewed changes

src/coreclr/jit/codegenwasm.cpp Show resolved Hide resolved

kg approved these changes Mar 5, 2026

View reviewed changes

use NONFAULTING

9d58a03

Copilot AI review requested due to automatic review settings March 5, 2026 22:07

Copilot started reviewing on behalf of AndyAyersMS March 5, 2026 22:08 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

SingleAccretion approved these changes Mar 6, 2026

View reviewed changes

AndyAyersMS merged commit dd2f6d3 into dotnet:main Mar 6, 2026
135 checks passed

dotnet-maestro bot mentioned this pull request Mar 7, 2026

[main] Source code updates from dotnet/runtime dotnet/dotnet#5301

Merged

Conversation

AndyAyersMS commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndyAyersMS commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dotnet-policy-service bot commented Mar 4, 2026

Uh oh!

dotnet-policy-service bot commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

SingleAccretion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

AndyAyersMS commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

AndyAyersMS commented Mar 3, 2026 •

edited

Loading