[wasm] Use ordinary struct alignment for Int128/UInt128 on wasm

[lewing]

Int128/UInt128 were given a 16-byte alignment requirement on wasm by both the VM and crossgen2, annotated // sizeof(v128). Int128 is not a v128 — the only 128-bit value type in WebAssembly is v128, and WasmLowering.IsWasmV128Type recognizes only Vector128<T> and a 128-bit Vector<T>, so Int128 uses the generic by-reference struct ABI.

Every other alignment override in CheckForSystemTypes exists because the managed type corresponds to a fundamental data type in the target ABI (Vector128<T>↔__m128, and so on). Int128 has no counterpart on wasm, so it takes ordinary struct alignment — the 8 its two ulong fields produce. On the crossgen2 side that means routing wasm through the existing 32-bit path alongside ARM.

Symptom

The 16 was observable as wrong codegen at the interpreter→R2R boundary:

Int128 a = 1, b = 1;
a == b          // True
a.Equals(b)     // False   <-- R2R only
a.CompareTo(b)  // -1      <-- R2R only

Root cause

Wasm signature encoding spells a by-reference struct argument as S<N>, and RaiseSignature resolves S16 through GetCachedStructOfSize, which keys only on size and keeps the first struct seen at that size. The thunk therefore used that struct's 8-byte alignment while the interpreter used Int128's 16, placing the argument at argsBase+24 instead of argsBase+32. Equals and CompareTo read a shifted value; operator == takes its operands differently and was unaffected.

This also makes S<N> sound rather than accidentally correct. Single-field structs wrapping a v128 are unwrapped by LowerType and encode as V, and a v128 field is the only source of 16-byte alignment on wasm — so once Int128 takes its natural alignment, nothing reaching S<N> requires more than 8.

The VM and crossgen2 must change together or managed field layout desyncs.

Validation

Browser wasm, against builds with and without the change, rebuilding both corerun and the R2R composite for each variant:

	R2R off	R2R on
Int128 repro, before	correct	Equals=False, CompareTo=±1 on equal values
Int128 repro, after	correct	correct

System.Text.Json, the Int128-bearing classes, 1496 tests, run on a branch carrying the other outstanding wasm R2R fixes so the suite completes:

	R2R off	R2R on
before	2 failed (0 Int128)	20 failed (15 Int128)
after	2 failed (0 Int128)	2 failed (0 Int128)

Those 2 fail with R2R off in both arms — pre-existing and unrelated.

Microsoft.Bcl.Memory 550/550 and System.Runtime.CompilerServices.Unsafe 128/128, R2R on and off.

Alternative considered

Encoding Int128 as V instead does not work. V is not an alignment tag — it selects a different calling convention, passing the value in a wasm v128 local rather than by reference. The JIT has no notion of Int128 at all, so R2R code faults with RuntimeError: function signature mismatch. Confirmed experimentally.

clang on wasm32 agrees with the by-reference treatment: it legalizes __int128 into two i64 parameters rather than any 128-bit entity, and passes an equivalent 16-byte struct as byval align 8. _Alignof(__int128) == 16 there is a memory-layout artifact, not a calling-convention requirement, since there is no 128-bit slot to align to.

wasm-only by construction — the VM change is inside #elif defined(TARGET_WASM) and the crossgen2 change adds TargetArchitecture.Wasm32 to an existing condition.

Note

This pull request was created with the assistance of GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

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

[lewing]

I believe clang sets __int128 alignment to 16 and passes it as two 64 bit args so there is an argument to be made for special handling here but that change is larger and there is no current consumer.

[lewing]

cc @dotnet/wasm-contrib

[Copilot]

Pull request overview

This PR changes how System.Int128/System.UInt128 alignment is treated on WebAssembly so that wasm uses ordinary struct alignment (8) instead of a forced 16-byte alignment, keeping VM and crossgen2/R2R field layout consistent and avoiding ABI mismatches at interpreter↔R2R boundaries.

Changes:

• In CoreCLR VM CheckForSystemTypes, switch wasm Int128/UInt128 alignment requirement from 16 to 8.
• In crossgen2 field layout, treat TargetArchitecture.Wasm32 like existing 32-bit ARM behavior by using the standard metadata layout engine for Int128/UInt128.

Reviewed changes

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

File	Description
src/coreclr/vm/methodtablebuilder.cpp	Updates wasm Int128/UInt128 alignment requirement to 8 in system type layout overrides.
src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs	Routes wasm32 through the “32-bit metadata layout” path to match VM layout and avoid 16-byte override.

[lewing]

Correct that the comment is imprecise — X86 is 32-bit and does take the 16-byte override. That imprecision predates this change though, and the condition it describes is right there on the next line, so I'd rather not expand it here: rewording a pre-existing comment is unrelated churn, and per repo convention comments are kept minimal because inaccurate ones mislead and need maintenance.

Happy to fix it if a maintainer would prefer.

Note

This comment was generated by GitHub Copilot.

[lewing]

Added Wasm32 cases to TestAlignmentBehavior_AutoAlignmentRulesWithOSDependence in 5fe365d. Both fail without the layout change (alignment 16 rather than 8), so they actually guard the new branch rather than just documenting it.

Note

This comment was generated by GitHub Copilot.

[Copilot]

Pull request overview

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

[tannergooding]

I believe clang sets __int128 alignment to 16 and passes it as two 64 bit args so there is an argument to be made for special handling here but that change is larger and there is no current consumer.

This impacts packing and layout, something we really need to have correct from v1 as it can be breaking (behavioral, impact interop, invalidate R2R images, etc) to change it later.

The general tooling docs describing the basic C ABI (not required, just existing tooling convention) explicitly calls out 16: https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md#data-representation

emscripten then currently chooses 16-byte alignment/packing as well, in part because LLVM doesn't override it: https://github.com/llvm/llvm-project/blob/6baa9c9ac22c60e00471c6412387b5a48aa12e3e/clang/lib/Basic/Targets/WebAssembly.h#L28-L92 and https://github.com/llvm/llvm-project/blob/6baa9c9ac22c60e00471c6412387b5a48aa12e3e/llvm/lib/TargetParser/TargetDataLayout.cpp#L523-L533

Rust notably had a similar bug where they didn't use 16-byte alignment as expected and had to fix it last year: rust-lang/rust#133991

So I don't think we can take this and rather need to ensure the interpreter correctly handles this for stack data.

[lewing]

we have the same problem for v256/512, for which there are no intrinsic types either

[tannergooding]

we have the same problem for v256/512, for which there is no intrinsic type either

Those ones will require every ecosystem to take a break though, if WASM adds official types in the future. As of now, the layout/alignment should be correct as they should be recognized as 2 or 4x V128 fields

I think it's quite a bit different from Int128 which already has a defined contract today and where it will impact layout/interop with other targets.

Is getting the interpreter to handle the 16-byte stack alignment required here (and for Decimal128 which has its own PR up fixing layout on all targets, not just WASM), that much more complex?

[lewing]

You're right, and the BasicCABI table refutes my reasoning rather than just outweighing it. The row is:

General type	C Type	sizeof	Alignment	Wasm Value Type
Integer	__int128_t	16	16	(none)

My argument was "there is no corresponding wasm value type, therefore no ABI basis for the alignment override." The spec assigns alignment 16 to a type whose value type is explicitly (none), so that inference doesn't hold, and layout is a contract independently of how arguments are passed. Withdrawing this.

On the interpreter question: the interpreter looks like the side that's already correct. Tracing the original failure, it placed the Int128 argument at argsBase+32 (16-aligned); the R2R thunk placed it at argsBase+24. The thunk isn't consulting the type — LowerSignature spells a by-reference aggregate as S<N> with N = size only, and RaiseSignature turns S16 back into a type via GetCachedStructOfSize, which keys on size alone and keeps whichever 16-byte struct was lowered first.

Teaching S<N> about alignment would make the two agree, but it would still be the wrong calling convention: clang passes __int128 as two i64s by value, and a 16-byte struct as one i32 pointer.

callee    (long long, __int128,            int) -> (i32, i64, i64, i64, i32)
callee_s16(long long, struct{ulong,ulong}, int) -> (i32, i64, i32, i32)

That divergence is reachable rather than theoretical, since LowerSignature also drives IsUnmanagedCallersOnly. So Int128 likely needs its own signature encoding rather than reusing S<N> — closer to what V does for v128, though the wasm-level lowering would also need JIT support, as Int128 currently appears nowhere in src/coreclr/jit/. Still working that out.

Separately, the signature round-trip loses alignment for the vector types too, and that one is independent of whatever we decide for Int128. Reproduced on current main with no runtime changes, in a library crossgen'd for wasm where an 8-aligned 32-byte struct is lowered before the vector:

struct Blob32 { long a, b, c, d; }                       // 32 bytes, align 8  -> S32
static int Sum256(long tag, Vector256<int> v, int t)     // 32 bytes, align 32 -> S32
static int Sum512(long tag, Vector512<int> v, int t)     // 64 bytes, align 64 -> S64

R2R off:  Blob32 OK   Blob64 OK   Vector256 OK               Vector512 OK
R2R on:   Blob32 OK   Blob64 OK   Vector256 got=1002/1018    Vector512 got=1002/1026

Thunk signatures read out of the R2R image show the collision — Blob32 and Vector256<int> both emit S32:

WasmInterpreterToR2RThunk(llS32ip)   // Blob32
WasmInterpreterToR2RThunk(ilS32ip)   // Vector256<int>

Agreed no ecosystem contract is at stake for those yet. Worth noting the alignment isn't currently what you'd expect from 2x/4x V128 though: wasm has no branch in the Vector256/Vector512 handling in either CheckForSystemTypes or VectorFieldLayoutAlgorithm, so it falls to the final else and gets 32 and 64 rather than 16.

Note

This comment was generated by GitHub Copilot.

[lewing]

we have the same problem for v256/512, for which there is no intrinsic type either

Those ones will require every ecosystem to take a break though, if WASM adds official types in the future. As of now, the layout/alignment should be correct as they should be recognized as 2 or 4x V128 fields

I think it's quite a bit different from Int128 which already has a defined contract today and where it will impact layout/interop with other targets.

Is getting the interpreter to handle the 16-byte stack alignment required here (and for Decimal128 which has its own PR up fixing layout on all targets, not just WASM), that much more complex?

it requires another type signature I think, probably not huge but let me look.

[lewing]

we have the same problem for v256/512, for which there is no intrinsic type either

Those ones will require every ecosystem to take a break though, if WASM adds official types in the future. As of now, the layout/alignment should be correct as they should be recognized as 2 or 4x V128 fields

I think it's quite a bit different from Int128 which already has a defined contract today and where it will impact layout/interop with other targets.

Is getting the interpreter to handle the 16-byte stack alignment required here (and for Decimal128 which has its own PR up fixing layout on all targets, not just WASM), that much more complex?

My point was that they are also currently broken and have 32/64 alignment which probably doesn't make sense for the current wasm abi. So the question becomes how should we pass them? multiple v128s, or as a struct?

[tannergooding]

My point was that they are also currently broken and have 32/64 alignment which probably doesn't make sense for the current wasm abi. So the question becomes how should we pass them? multiple v128s, or as a struct?

They are defined as struct V256 { V128 lower, V128 upper } and struct V512 { V256 lower, V256 upper } and should be treated the same as equivalent user-defined structs (which is likewise how Arm64 treats them).

So a struct containing 2x V128 fields and a struct containing 2x structs, each containing 2x V128 fields. -- I expect WASM treats this the same as a struct containing 2x and 4x V128 fields, respectively. But some ABIs and our own layout algorithm/recursion should already be taking that into account when reporting things back.

[lewing]

My point was that they are also currently broken and have 32/64 alignment which probably doesn't make sense for the current wasm abi. So the question becomes how should we pass them? multiple v128s, or as a struct?

They are defined as struct V256 { V128 lower, V128 upper } and struct V512 { V256 lower, V256 upper } and should be treated the same as equivalent user-defined structs (which is likewise how Arm64 treats them).

So a struct containing 2x V128 fields and a struct containing 2x structs, each containing 2x V128 fields. -- I expect WASM treats this the same as a struct containing 2x and 4x V128 fields, respectively. But some ABIs and our own layout algorithm/recursion should already be taking that into account when reporting things back.

Right, I'm telling you the current handling is active broken because there is an alignment mismatch.

[tannergooding]

Right, I got that. I was explaining what the expected behavior is. If its incorrect today, then we probably need to fix it (I have no preference on whether that's the same or a separate PR)

[lewing]

Working on a separate prototype that can express the alignment and decomposition that matches the abi in the call signature without adding entirely new arg types.