[Wasm RyuJIT] "Partial containment" for GT_LCL_ADDR by kg · Pull Request #128680 · dotnet/runtime

kg · 2026-05-28T05:34:31Z

Experimenting with an approach for folding offsets into GT_IND and GT_STOREIND. Seems to work so far and doesn't complicate the code much, but the need to change codegenwasm.cpp:702 makes me wonder if this approach is fundamentally doomed...

cc @AndyAyersMS @SingleAccretion thoughts? I found a comment somewhere suggesting we need a new "nuw" frontend node but I'm not sure what a nuw is and whether that is the right solution for these scenarios.

LEAs are the next candidate but they seem trickier to get right. I'm not sure how common 'partially containable' LEAs and LCL_ADDRs are in a larger codebase, might do some instrumented runs over SPC to find out.

dotnet-policy-service · 2026-05-28T05:35:26Z

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 experiments with Wasm RyuJIT containment for GT_LCL_ADDR under indirect loads/stores, folding local frame offsets into Wasm memory operands while still emitting the frame pointer push.

Changes:

Marks GT_LCL_ADDR addresses of indirections as contained in Wasm lowering.
Adjusts Wasm codegen so contained GT_LCL_ADDR still emits its base address but skips the explicit offset add.
Emits folded offsets for GT_IND and GT_STOREIND memory operands.

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`	Adds containment marking for `GT_LCL_ADDR` operands of indirections.
`src/coreclr/jit/codegenwasm.cpp`	Implements partial-contained `GT_LCL_ADDR` codegen and folded load/store offsets.

…irections

SingleAccretion · 2026-05-28T10:19:27Z

thoughts?

I don't think we need to break the lowering-codegen contract to get this optimization. My thinking on it was transforming:

l = LCL_VAR_ADDR [+8]
    ...
    STOREIND(l, ...)

into:

l = LCL_VAR_ADDR [+0]
    ...
    LEA(l) [+8] ; contained
    STOREIND(l, ...)

there are some tweaks needed to make this work in liveness and things like VisitLocalDefs, since this store form would be 'new' for tracked (but on-stack) locals. But otherwise it's not controversial / tricky.

BTW, this is only a problem for STOREIND (and other stores), since for loads we can just contain directly.

"nuw" frontend node but I'm not sure what a nuw is

"No unsigned wrap" (LLVM terminology). The basic problem is avoid transforming this:

// p is byte*, consider "p == nuint.MaxValue - 2"
*(p + 3)

into a WASM address mode.

kg · 2026-05-28T17:13:51Z

thoughts?

I don't think we need to break the lowering-codegen contract to get this optimization. My thinking on it was transforming:
l = LCL_VAR_ADDR [+8]
    ...
    STOREIND(l, ...)
into:
l = LCL_VAR_ADDR [+0]
    ...
    LEA(l) [+8] ; contained
    STOREIND(l, ...)
there are some tweaks needed to make this work in liveness and things like VisitLocalDefs, since this store form would be 'new' for tracked (but on-stack) locals. But otherwise it's not controversial / tricky.

The tweaks are because this would 'erase' the reference to the var at +8 right, by replacing it with +0 +8? So the reference to var2 disappears and becomes a false reference to var0.

I'm not sure I like that...

BTW, this is only a problem for STOREIND (and other stores), since for loads we can just contain directly.

"nuw" frontend node but I'm not sure what a nuw is

"No unsigned wrap" (LLVM terminology). The basic problem is avoid transforming this:
// p is byte*, consider "p == nuint.MaxValue - 2"
*(p + 3)
into a WASM address mode.

SingleAccretion · 2026-05-28T17:36:58Z

The tweaks are because this would 'erase' the reference to the var at +8 right, by replacing it with +0 +8? So the reference to var2 disappears and becomes a false reference to var0.

It needs a bit of thinking around how to represent the frame base while still referencing the original local (it's not negotiable the original local needs to stay referenced, of course). The saving grace I think is that we only need this new contract for the very small window from RA to codegen.

The easiest (but not the most efficient) way is to introduce a new LEA-like node that'd have a contained LCL_ADDR and another operand as just the frame base (e. g. PHYS_REG), like so:

STOREIND
  NEW_NODE ; contained
    LCL_ADDR ; contained, for liveness
    PHYS_REG <frame base> ; not contained

it looks wacky though, maybe there are better idea out there.

Copilot

Pull request overview

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

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

kg · 2026-05-30T07:36:24Z

I'm hitting this assertion now that I added the containment check copilot suggested:

Single method repro args:--singlemethodtypename "Microsoft.Win32.SafeHandles.SafeFileHandle" --singlemethodname "CreateAnonymousPipe" --singlemethodindex 1
Z:\runtime\src\coreclr\jit\lower.cpp:12013
Assertion failed 'm_compiler->lvaGetDesc(lclAddr)->IsAddressExposed()' in 'Microsoft.Win32.SafeHandles.SafeFileHandle:CreateAnonymousPipe(byref,byref,bool,bool)' during 'Linear scan register alloc' (IL size 160; hash 0x22c9c037; MinOpts)

Not sure what's going on, but will keep digging...

Copilot

Pull request overview

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

kg · 2026-05-30T08:57:25Z

+            case GT_PARTIALLY_CONTAINED_LCL_ADDR:
+                // WASM-TODO: Implement removal for dead partially contained lcl_addrs and their containers
+                if (TLiveness::EliminateDeadCode && node->IsUnusedValue())
+                {
+                    JITDUMP("WASM-TODO: Not removing unused partially contained local address:");
+                    DISPNODE(node);
+                }
+                break;


This isn't hit at all during a CG2 of S.P.CoreLib for me so it may be a theoretical problem

+    if (addr->OperIs(GT_LCL_ADDR) && IsContainableLclAddr(addr->AsLclFld(), indirNode->Size() DEBUGARG(true /* disableAssertion */)))
+    {
+        // An indir through a lcl_addr should never have a multiply-used address since it doesn't need a null check
+        //  and can't fault. If it does, this transform is incorrect.
+        assert((addr->gtLIRFlags & LIR::Flags::MultiplyUsed) == LIR::Flags::None);


Copilot AI review requested due to automatic review settings May 28, 2026 05:34

kg added arch-wasm WebAssembly architecture area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels May 28, 2026

Copilot started reviewing on behalf of kg May 28, 2026 05:34 View session

dotnet-policy-service Bot assigned kg May 28, 2026

Copilot AI reviewed May 28, 2026

View reviewed changes

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

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

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

kg added 2 commits May 28, 2026 02:35

Checkpoint: "Partial containment" for GT_LCL_ADDR nodes inside of ind…

a518b36

…irections

Refactoring

963c7fa

Checkpoint

4a4b002

kg force-pushed the wasm-lcladdr-partial-containment branch from e27c7b7 to 4a4b002 Compare May 29, 2026 13:26

Cleanup

d9fed2c

Copilot AI review requested due to automatic review settings May 29, 2026 13:30

Copilot started reviewing on behalf of kg May 29, 2026 13:30 View session

Copilot AI reviewed May 29, 2026

View reviewed changes

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

Comment thread src/coreclr/jit/codegenwasm.cpp

Potential fix for pull request finding

6541553

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings May 29, 2026 13:38

Copilot started reviewing on behalf of kg May 29, 2026 13:38 View session

Address PR feedback

ecb38d2

Copilot AI reviewed May 29, 2026

View reviewed changes

Comment thread src/coreclr/jit/lowerwasm.cpp

Comment thread src/coreclr/jit/codegenwasm.cpp

kg added 2 commits May 30, 2026 00:18

Cleanup

29b5b05

Improve disassembly quality

ede962e

Allow disabling nuisance assertion in IsContainableLclAddr

9fbd3d0

Copilot AI review requested due to automatic review settings May 30, 2026 07:57

Copilot started reviewing on behalf of kg May 30, 2026 07:57 View session

Copilot AI reviewed May 30, 2026

View reviewed changes

kg added 2 commits May 30, 2026 01:30

Update gcWriteBarrierFormFromTargetAddress

2b2932e

Update some relevant code

bd05ac2

Conversation

kg commented May 28, 2026

Uh oh!

dotnet-policy-service Bot commented May 28, 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!

SingleAccretion commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kg commented May 28, 2026

Uh oh!

SingleAccretion commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

kg commented May 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

kg May 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SingleAccretion commented May 28, 2026 •

edited

Loading

SingleAccretion commented May 28, 2026 •

edited

Loading