Skip to content

[Wasm RyuJIT] Block stores#123738

Merged
kg merged 11 commits intodotnet:mainfrom
kg:wasm-blockstore
Feb 23, 2026
Merged

[Wasm RyuJIT] Block stores#123738
kg merged 11 commits intodotnet:mainfrom
kg:wasm-blockstore

Conversation

@kg
Copy link
Member

@kg kg commented Jan 28, 2026

This is sufficient to compile these three managed methods to wasm functions with native memory.fill and memory.copy instructions:

    struct S {
        public int A, B, C, D;
    }

    struct S2 {
        public string A, B, C, D;
    }

    [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
    static unsafe void zeroStruct (S *result) {
        *result = default;
    }

    [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
    static unsafe void copyStruct (S *a, S *b) {
        *a = *b;
    }

    [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
    static void zeroStructWithRefs (ref S2 result) {
        result = default;
    }

@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 Jan 28, 2026
@dotnet-policy-service
Copy link
Contributor

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

@kg
Copy link
Member Author

kg commented Jan 28, 2026

Not sure what i'm doing wrong here yet.


*************** Starting PHASE Lowering nodeinfo
compEnregLocals() is false, setting doNotEnreg flag for all locals.
Local V00 should not be enregistered because: opts.compFlags & CLFLG_REGVAR is not set

Local V01 should not be enregistered because: opts.compFlags & CLFLG_REGVAR is not set

Local V02 should not be enregistered because: opts.compFlags & CLFLG_REGVAR is not set

Local V03 should not be enregistered because: opts.compFlags & CLFLG_REGVAR is not set
BB02: already in WASM value stack order
lowering store lcl var/field (before):
N001 (???,???) [000002] -----+-----                    t2 =    CNS_INT   int    0
                                                            /--*  t2     int    
N002 (???,???) [000003] DA---+-----                         *  STORE_LCL_VAR struct<Program+S, 16> V03 loc1         


Local V03 should not be enregistered because: written/read in a block op
lowering store lcl var/field (after):
N001 (???,???) [000002] -----+-----                    t2 =    CNS_INT   int    0
               [000012] D----------                   t12 =    LCL_ADDR  byref  V03 loc1         [+0]
                                                            /--*  t12    byref  
                                                            +--*  t2     int    
N002 (???,???) [000003] sA---+-----                         *  STORE_BLK struct<Program+S, 16> (init)


Local V03 should not be enregistered because: written/read in a block op
lowering return node
N001 (???,???) [000007] -----+-----                         *  RETURN    void  
============


Program:copyStruct(ptr) - NYI (Z:\runtime\src\coreclr\jit\lowerwasm.cpp:476 - NYI_WASM: IR not in a stackified form)
Z:\runtime\src\coreclr\jit\lowerwasm.cpp:476
Assertion failed 'NYI_WASM: IR not in a stackified form' in 'Program:copyStruct(ptr)' during 'Lowering nodeinfo' (IL size 17; hash 0xce1e49ee; MinOpts)

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements infrastructure for WebAssembly block store operations in the RyuJIT compiler. It adds support for lowering block copy and initialization operations to use WASM-specific memory.copy and memory.fill instructions.

Changes:

  • Adds LowerBlockStore implementation for WASM target to handle block copy and initialization operations
  • Introduces two new WASM instructions (memory_copy and memory_fill) and their associated instruction format (IF_MEMCPY)
  • Adds emitter support for encoding and displaying the new IF_MEMCPY instruction format

Reviewed changes

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

File Description
src/coreclr/jit/lowerwasm.cpp Implements LowerBlockStore to handle GT_STORE_BLK and GT_INIT_BLK operations, with partial infrastructure for memory.copy and memory.fill
src/coreclr/jit/instrswasm.h Adds memory_copy and memory_fill instruction definitions with their opcodes
src/coreclr/jit/emitfmtswasm.h Defines the IF_MEMCPY instruction format for memory operations with two memory indices
src/coreclr/jit/emitwasm.cpp Implements encoding, size calculation, and display logic for IF_MEMCPY instruction format

Copy link
Contributor

Copilot AI left a comment

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 8 out of 8 changed files in this pull request and generated 4 comments.

Copilot AI review requested due to automatic review settings February 18, 2026 06:01
Copy link
Contributor

Copilot AI left a comment

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 8 out of 8 changed files in this pull request and generated no new comments.

@kg
Copy link
Member Author

kg commented Feb 18, 2026

cc @adamperlin see the WasmLowering.GetSignature changes. I couldn't figure out what was wrong with the existing implementation so I rewrote it, but the rewrite isn't complete. It was generating signatures with zeroes in them and that produced invalid wasm modules.

@AndyAyersMS
Copy link
Member

I couldn't figure out what was wrong with the existing implementation so I rewrote it, but the rewrite isn't complete. It was generating signatures with zeroes in them and that produced invalid wasm modules.

That's probably my fault: I revised that code to try and handle the return buffer placement.

@MichalStrehovsky in the managed type system signatures is the this always explicitly represented?

@MichalStrehovsky
Copy link
Member

@MichalStrehovsky in the managed type system signatures is the this always explicitly represented?

If you mean the MethodSignature class, it represents what the input ECMA-335 has. So:

  • !MethodSignature.IsStatic && !MethodSignature.IsExplicitThis: there's an implicit this parameter not listed in the signature
  • !MethodSignature.IsStatic && MethodSignature.IsExplicitThis: this is part of the signature (can be hit in obscure cases involving C++/CLI when the compiler wants to attach modifiers to this, likely not a concern for WASM targets)
  • MethodSignature.IsStatic: static method, no this for obvious reasons

@adamperlin
Copy link
Contributor

adamperlin commented Feb 19, 2026

cc @adamperlin see the WasmLowering.GetSignature changes. I couldn't figure out what was wrong with the existing implementation so I rewrote it, but the rewrite isn't complete. It was generating signatures with zeroes in them and that produced invalid wasm modules.

The re-write looks good to me, appending to a list seems like an easier solution to ensure correctness. My only feedback would just be that we should probably add a comment (maybe in the header) explaining what the maximum number of parameters we might add when lowering the signature is (explaining the + 4 in the allocated signature list size).

Copilot AI review requested due to automatic review settings February 19, 2026 22:31
Copy link
Contributor

Copilot AI left a comment

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 8 out of 8 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/coreclr/jit/lowerwasm.cpp:484

  • The DEBUG logging should use JITDUMP/JITDUMPEXEC instead of raw printf to be consistent with existing JIT logging patterns. The suggested approach is to use JITDUMP("node==[%06u] prev==[%06u]\n", Compiler::dspTreeID(node), Compiler::dspTreeID(prev)); which eliminates the need for the #ifdef DEBUG wrapper.

                    JITDUMP("node==[%06u] prev==[%06u]\n", Compiler::dspTreeID(node), Compiler::dspTreeID(prev));
                    NYI_WASM("IR not in a stackified form");
                }

                // In stack order, the last operand is closest to its parent, thus put on top here.
                node->VisitOperands([stack](GenTree* operand) {
                    stack->Push(operand);
                    return GenTree::VisitResult::Continue;
                });

@kg kg marked this pull request as ready for review February 19, 2026 23:06
@kg
Copy link
Member Author

kg commented Feb 19, 2026

@dotnet/jit-contrib PTAL

Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

Looks good overall, just curious about the GC disabling part.

kg added 4 commits February 19, 2026 19:24
Add insns for memory copy and fill

Remove NYIs so stuff can flow through

Cleanup

Checkpoint

Fix opcodes

Fix WasmLowering.GetSignature

Maybe-working codegen for zeroing a struct

jit-format

Remove unused local

Consume operands

Fix erroneous fallthrough

Address PR feedback

Remove dead call

NotImplementedException

ArrayBuilder and comment

Make initblk loop work
Remove else case
@kg kg force-pushed the wasm-blockstore branch from ab3fa66 to b9ac6d9 Compare February 20, 2026 08:43
@SingleAccretion SingleAccretion self-requested a review February 20, 2026 13:38
Copilot AI review requested due to automatic review settings February 21, 2026 02:41
kg and others added 3 commits February 20, 2026 18:41
Copy link
Contributor

Copilot AI left a comment

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 8 out of 8 changed files in this pull request and generated 3 comments.

Copilot AI review requested due to automatic review settings February 21, 2026 04:04
Copy link
Contributor

Copilot AI left a comment

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 8 out of 8 changed files in this pull request and generated 2 comments.

@kg
Copy link
Member Author

kg commented Feb 23, 2026

Resolving all comments to merge as-is. Issues will be fixed in follow-up PRs.

@kg kg merged commit e5f7893 into dotnet:main Feb 23, 2026
126 of 128 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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.

7 participants