[Wasm RyuJIT] Address block store issues from #125756 by kg · Pull Request #125770 · dotnet/runtime

kg · 2026-03-19T11:58:10Z

Fixes some of the issues from #125756 , at least based on my isolated repros.

cc @AndyAyersMS still figuring this out, don't completely understand it.

dotnet-policy-service · 2026-03-19T11:59:01Z

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

Copilot

Pull request overview

This PR targets WebAssembly RyuJIT block-store lowering/codegen issues that are triggering asserts during crossgen/debug builds (per #125756), by adjusting how GT_STORE_BLK operands are represented/contained for memory.copy/memory.fill.

Changes:

Update WASM LowerBlockStore to treat GT_IND sources as contained, and attempt to convert local sources to addresses for the memory.copy opcode path.
Add debug assertions in WASM codegen to validate containment expectations for native bulk-memory op emission (memory.copy / memory.fill).

Reviewed changes

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

File	Description
src/coreclr/jit/lowerwasm.cpp	Changes block-store source handling/containment and attempts to convert local sources into address form for `memory.copy`.
src/coreclr/jit/codegenwasm.cpp	Adds debug assertions around contained operands for the native bulk-memory opcode path.

You can also share your feedback on Copilot code review. Take the survey.

src/coreclr/jit/lowerwasm.cpp

src/coreclr/jit/codegenwasm.cpp

AndyAyersMS · 2026-03-20T14:35:23Z

src/coreclr/jit/codegenwasm.cpp

+        }
+        else
+        {
+            assert(operand->OperIs(GT_LCL_FLD, GT_LCL_VAR, GT_LCL_ADDR));


GT_LCL_ADDR is here for dests?

Yeah when I expanded the cases this got called in, this assert tripped up on a LCL_ADDR at least once. Conceptually it seems reasonable to get an address here.

src/coreclr/jit/codegenwasm.cpp

AndyAyersMS

This looks good to me overall.

Copilot

Pull request overview

This PR targets WASM RyuJIT block-store correctness by adjusting lowering/RA hints and consolidating codegen paths for different GT_STORE_BLK lowering kinds, aiming to address debug/assert failures reported in #125756.

Changes:

Move SetMultiplyUsed marking in LowerBlockStore so it applies consistently across block-store kinds.
Route BlkOpKindNativeOpcode block stores through genCodeForCpObj to share logic with the CpObjUnroll path.
Refactor genCodeForCpObj operand handling to support both cpobj and native memory.copy/memory.fill.

Reviewed changes

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

File	Description
src/coreclr/jit/lowerwasm.cpp	Moves multiply-used marking to apply across cpobj and native-op block stores.
src/coreclr/jit/codegenwasm.cpp	Unifies `NativeOpcode` handling with `CpObjUnroll` and refactors operand decoding/emission.

Copilot · 2026-03-20T15:05:09Z

src/coreclr/jit/codegenwasm.cpp

+        else if (isSource && operand->OperIs(GT_CNS_INT))
+        {
+            addrType = TYP_INT;
+            offset   = 0;
+            reg      = REG_NA;
+        }
+        else
+        {
+            assert(operand->OperIs(GT_LCL_FLD, GT_LCL_VAR, GT_LCL_ADDR));
+            GenTreeLclVarCommon* lclVar = operand->AsLclVarCommon();


makeOperandRec doesn't handle initblk sources wrapped in GT_INIT_VAL (used to represent byte-pattern fills). For BlkOpKindNativeOpcode+initblk, cpObjNode->Data() can be GT_INIT_VAL, which will currently fall into the LCL_* branch and hit the assert / invalid AsLclVarCommon cast. Consider unwrapping GT_INIT_VAL to its operand before the GT_IND/GT_CNS_INT/LCL_* classification (similar to how other targets/codepaths drop GT_INIT_VAL).

Copilot · 2026-03-20T15:05:09Z

src/coreclr/jit/codegenwasm.cpp

+            bool                 fpBased;
+            reg    = GetFramePointerReg();
+            offset = m_compiler->lvaFrameAddress(lclVar->GetLclNum(), &fpBased) + lclVar->GetLclOffs();
+            assert(fpBased);


The LCL_* case in makeOperandRec computes an address using frame pointer + lvaFrameAddress, which is only valid when the local itself is the source buffer living on the stack. For destination address operands (and for byref locals that are enregistered), this computes the address of the local’s home/slot rather than the address value, and can also assert if the local isn’t FP-based. Consider using GetMultiUseOperandReg/operand->GetRegNum() for address-valued operands, and reserving the FP+offset computation only for stack-resident struct sources.

Suggested change

bool fpBased;

reg = GetFramePointerReg();

offset = m_compiler->lvaFrameAddress(lclVar->GetLclNum(), &fpBased) + lclVar->GetLclOffs();

assert(fpBased);

// For address-valued locals (e.g. GT_LCL_ADDR) or destination operands, the

// address is already materialized in a register; use that directly.

if (operand->OperIs(GT_LCL_ADDR) || !isSource)

{

addrType = operand->TypeGet();

reg = GetMultiUseOperandReg(operand);

offset = 0;

}

else

{

// For source locals that represent stack-resident buffers, compute

// the address as frame-pointer + frame offset.

bool fpBased;

reg = GetFramePointerReg();

offset = m_compiler->lvaFrameAddress(lclVar->GetLclNum(), &fpBased) + lclVar->GetLclOffs();

assert(fpBased);

}

Is this correct? I don't really understand it

There are quite a few different cases and it's hard to be sure without tracing through what happens upstream.

You might want to work through a set of examples just to see the variety of things that need to be handled. Something like:

struct S { ... } S getS(); void useS(ref S s); void foo(ref S src, ref S dst) { dst = src } void foo(S src, ref S dst) { dst = src } void foo(ref S src) { S dst = src; } void foo(ref S src) { useS(src); } void foo (ref S dst) { S src = new S(...); dst = src } void foo (ref S dst) { S src = new S; dst = src } void foo (ref S dst) { dst = getS(); }

and try cases where S is a single-field wrapper and also has multiple fields.

src/coreclr/jit/codegenwasm.cpp

SingleAccretion · 2026-03-20T16:38:30Z

src/coreclr/jit/lowerwasm.cpp

            blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindNativeOpcode;
        }
+
+        SetMultiplyUsed(dstAddr);


Checks missing for whether this actually needs the null check (GTF_IND_NONFAULTING).

Also, since we're fixing this code, may as well fix it completely by adding the null check for init blocks too.

SingleAccretion · 2026-03-20T16:41:06Z

src/coreclr/jit/codegenwasm.cpp


 //------------------------------------------------------------------------
-// genCodeForCpObj: Produce code for a GT_STORE_BLK node that represents a cpobj operation.
+// genCodeForCpObj: Produce code for a GT_STORE_BLK node that represents a cpobj operation


There is no specific need to overload genCodeForCpObj... we can just house all this logic in genCodeForStoreBlk if it makes things easier to share. It actually looks like it would flush out the logic to look more straight as well.

SingleAccretion · 2026-03-20T16:52:21Z

src/coreclr/jit/codegenwasm.cpp

+        bool      isContained;
+    };
+
+    auto makeOperandRec = [&](GenTree* operand, bool isSource) {


Using this for the destination looks unnecessary, since we don't need to materialize the destination address manually. The pattern we had here before was simpler. More generally it feels like open-coding the various possibilities would lead to simpler code.

SingleAccretion · 2026-03-20T16:53:13Z

src/coreclr/jit/codegenwasm.cpp

-        srcOffset   = 0;
-
-        if (doNullCheck)
+    struct operandRec


Naming nit: operandRec -> OperandRec, addrType -> AddrType.

(What does "rec" mean?)

kg added the arch-wasm WebAssembly architecture label Mar 19, 2026

Copilot AI review requested due to automatic review settings March 19, 2026 11:58

kg added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 19, 2026

dotnet-policy-service bot assigned kg Mar 19, 2026

Copilot AI reviewed Mar 19, 2026

View reviewed changes

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

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

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

SingleAccretion reviewed Mar 19, 2026

View reviewed changes

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

This was referenced Mar 19, 2026

osx builds failing with CMake error #125757

Closed

MsQuic fails with QUIC_STATUS_OUT_OF_MEMORY on AzureLinux #123216

Open

AndyAyersMS reviewed Mar 20, 2026

View reviewed changes

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

AndyAyersMS reviewed Mar 20, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings March 20, 2026 14:55

Copilot started reviewing on behalf of kg March 20, 2026 14:56 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

kg marked this pull request as ready for review March 20, 2026 15:54

kg added 4 commits March 20, 2026 08:56

Checkpoint

bbadda5

Checkpoint

e0d6720

Update comment & run jit-format

9f00bdb

Update comments

ae3f194

kg force-pushed the wasm-blockstore-4 branch from b5704e6 to ae3f194 Compare March 20, 2026 15:56

SingleAccretion reviewed Mar 20, 2026

View reviewed changes

Conversation

kg commented Mar 19, 2026

Uh oh!

dotnet-policy-service bot commented Mar 19, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AndyAyersMS left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants