Skip to content

[Wasm RyuJIT] Fix classifier embiggening small structs that are being passed-as a wasm primitive#125278

Open
kg wants to merge 3 commits intodotnet:mainfrom
kg:issue125199-1
Open

[Wasm RyuJIT] Fix classifier embiggening small structs that are being passed-as a wasm primitive#125278
kg wants to merge 3 commits intodotnet:mainfrom
kg:issue125199-1

Conversation

@kg
Copy link
Member

@kg kg commented Mar 6, 2026

Addresses part of #125199

@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 Mar 6, 2026
Copilot AI review requested due to automatic review settings March 6, 2026 20:30
@kg kg changed the title Fix classifier embiggening small structs that are being passed-as a wasm primitive [Wasm RyuJIT] Fix classifier embiggening small structs that are being passed-as a wasm primitive Mar 6, 2026
@dotnet-policy-service
Copy link
Contributor

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

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

Adjusts Wasm ABI classification for small structs that are lowered to Wasm primitive types to avoid size/type mismatches during local stack stores (part of #125199).

Changes:

  • Compute the ABI passing segment size for lowered structs using the struct’s actual size (capped by the ABI primitive size) instead of always using the primitive size.

@kg kg marked this pull request as ready for review March 7, 2026 00:26
Copilot AI review requested due to automatic review settings March 7, 2026 00:26
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 1 out of 1 changed files in this pull request and generated 1 comment.

regNumber reg = MakeWasmReg(m_localIndex++, genActualType(abiType));
// If the struct is being passed directly as a wasm value, make sure we record
// the actual size of the struct, not the size of the containing wasm value.
unsigned segmentSize = passByRef ? genTypeSize(abiType) : min(structLayout->GetSize(), genTypeSize(abiType));
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

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

When passByRef is false, using min(structLayout->GetSize(), genTypeSize(abiType)) can silently truncate the recorded segment size if getWasmLowering ever returns a wasm primitive that’s smaller than the actual struct size. That would mask a classifier/EE mismatch and could lead to incorrect copying later; consider using structLayout->GetSize() directly and adding a debug assert that it fits in genTypeSize(abiType) instead of clamping.

Suggested change
unsigned segmentSize = passByRef ? genTypeSize(abiType) : min(structLayout->GetSize(), genTypeSize(abiType));
unsigned abiTypeSize = genTypeSize(abiType);
unsigned segmentSize;
if (passByRef)
{
segmentSize = abiTypeSize;
}
else
{
unsigned structSize = structLayout->GetSize();
assert(structSize <= abiTypeSize);
segmentSize = structSize;
}

Copilot uses AI. Check for mistakes.
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.

2 participants