Uh oh!
There was an error while loading. Please reload this page.
[release/11.0-rc1] [mono][interp] Intrinsify Vector128.Store/StoreUnsafe on wasm - #132575
Merged
lewing merged 1 commit intoAug 20, 2026
Merged
Conversation
Rebased onto `main` now that #132500 has merged; this is a single commit touching 3 files. `Vector128.Store` and `Vector128.StoreUnsafe` were deliberately left out of the PackedSimd alias table because their operands are reversed relative to the method they would lower to: ``` PackedSimd.Store (T* address, Vector128<T> source) Vector128.Store (this Vector128<T> source, T* destination) ``` The alias path only renames the method, and `emit_common_simd_epilogue` assigns sregs in signature order, so aliasing them as-is would have passed the vector where the destination address is expected. They therefore fell back to managed code, which routes through `Unsafe.WriteUnaligned` — an intrinsic the interpreter does not implement — making a documented-as-fast API roughly six times slower than the PackedSimd equivalent. Add them to the alias table and swap the two sregs after the epilogue has run. sregs are var indices consumed positionally by `MINT_SIMD_INTRINS_P_PP` (`interp.c:6141`), so the swap is a permutation: it changes their order, not the set of vars used, leaving liveness and refcounting unaffected. It runs during IL→IR generation, before every optimization pass. It also matches what the jiterpreter already expects — `SimdIntrinsic3.StoreANY` in `jiterpreter-trace-generator.ts` loads arg 2 as the address and arg 3 as the vector. `Store` is registered for every element type (`ANY`), so the reorder is guarded on the shape actually being a store: two parameters, void return, and a raw address (`T*` or `ref T`) as the second parameter. The three-argument `StoreUnsafe(source, destination, elementOffset)` has no PackedSimd counterpart and is rejected by the parameter count, falling back to managed code as before. `System.Numerics.Vector.Store`/`StoreUnsafe` share the identical shape and route through the same emit path, so they are lowered too. `StoreAligned`/`StoreAlignedNonTemporal` are absent from the alias table and continue to fall back to managed code — importantly, since `StoreAligned` has a runtime alignment check that must throw. `interp_packedsimd_store` assigned through a `v128_t*`, claiming 16-byte alignment that neither `PackedSimd.Store` nor `Vector128.StoreUnsafe` guarantee. It now uses `wasm_v128_store`, which stores through a `__packed__ __may_alias__` struct. This mirrors `interp_packedsimd_load128`'s existing use of `wasm_v128_load`, and matches the jiterpreter's `v128_store` with an alignment hint of 1. Misaligned access is well-defined at the wasm ISA level, so this was C UB rather than a live bug. browser-wasm under V8, interpreter, 1,000,000 iterations, 2 warmups, best of 5, same harness before and after on the same machine: | Operation | Before | After | Speedup | |---|---|---|---| | `Vector128.Store<int>` | 3.243 ns | 0.521 ns | **6.2x** | | `Vector128.StoreUnsafe<int>` | 3.253 ns | 0.518 ns | **6.3x** | | `PackedSimd.Store<int>` (control) | 0.485 ns | 0.527 ns | unchanged | The control's ~8% drift bounds run-to-run noise; the 6x is far outside it. No loop-overhead baseline is subtracted — an empty-loop control measured *higher* than the store loops, since the jiterpreter doesn't trace it. - browser-wasm `mono+libs` Release: 0 errors, 0 warnings - native osx-arm64 `mono` Release: 0 errors, 0 warnings - `System.Runtime.Intrinsics`: **13024/13024** passing (13021 baseline + 3 new) - `System.Numerics.Vectors`: **7449/7449** passing - IR (`MONO_VERBOSE_METHOD`): `simd_intrins_p_pp [74 <- 73 72], 191` — descending sregs confirm the swap; the three-argument overload still emits a managed `call` - `System.Numerics.Vector<T>.StoreUnsafe` likewise emits `simd_intrins_p_pp [19 <- 18 14], 191` Three tests covering `Store`, `StoreUnsafe`, and `StoreUnsafe` with an element offset, across twelve element types, at every 16-byte alignment, with guard bytes on both sides so a misplaced or oversized store is caught. The element-offset arm uses `StoreUnsafe(ref *(destination - 1), elementOffset: 1)` so a dropped offset argument shows up as a misplaced store. They pass on an unmodified runtime as well, so they validate behavior rather than implementation. > [!NOTE] > This pull request was authored with the assistance of GitHub Copilot. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ebb2f49-c9fc-4c28-9755-c51b9deac735 (cherry picked from commit be1c680)
lewing
requested review from
BrzVlad, MichalStrehovsky, kotlarmilos, pavelsavara, steveisok and vitek-karas
as code ownersAugust 20, 2026 17:28
Contributor
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
|
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. |
steveisok
approved these changes
Aug 20, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR backports the Mono interpreter/WASM optimization to intrinsify Vector128.Store / Vector128.StoreUnsafe by routing them through the PackedSimd store intrinsic, including an operand swap to account for the reversed parameter ordering between the APIs. It also updates the WASM store helper to avoid C UB on unaligned destinations and adds regression tests to validate correct behavior across alignments and element types.
Changes:
- Add
Store/StoreUnsafeto the PackedSimd alias table and detect store-shaped calls that require swapping operands. - Swap the two operand sregs after
emit_common_simd_epilogueassigns them in signature order, so the interpreter passes(addr, vec)to the PackedSimd store helper. - Update
interp_packedsimd_storeto usewasm_v128_storefor well-defined unaligned stores, and add alignment-sensitive store tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/mono/mono/mini/interp/transform-simd.c | Adds Store/StoreUnsafe aliasing and swaps sregs for store-shaped signatures so the interpreter calls PackedSimd.Store with correct operand order. |
| src/mono/mono/mini/interp/interp-simd.c | Switches the PackedSimd store helper to wasm_v128_store to avoid assuming 16-byte alignment. |
| src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector128Tests.cs | Adds tests validating correct store behavior for unaligned destinations (including the elementOffset overload). |
lewing
commented
Aug 20, 2026
MemberAuthor
/ba-g failures are known, this is a release branch |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Manual backport of #132502 to
release/11.0-rc1.Why this needed a manual backport
The automatic cherry-pick conflicted in two files:
transform-simd.c— resolved by first landing the prerequisite, [release/11.0-rc1] [mono][interp] Fix mis-sorted SIMD intrinsic tables making entries unreachable #132525 (backport of [mono][interp] Fix mis-sorted SIMD intrinsic tables making entries unreachable #132500). Once that merged, this file auto-merged with no hand edits.Vector128Tests.cs— a pure placement conflict. [mono][interp] Intrinsify Vector128.Store/StoreUnsafe on wasm #132502's tests were appended afterVector128GetElementVariableOutOfRangeTest, which comes from [mono][interp] Intrinsify Vector128 GetElement/WithElement on wasm #132499 and is not on this branch. The three Store tests are self-contained, so they were appended at the end of the class instead. No [mono][interp] Intrinsify Vector128 GetElement/WithElement on wasm #132499 tests were pulled in.Every added and removed line in this commit is byte-identical to upstream
be1c680— only hunk headers and line numbers differ.Depends on #132525
This backport is only effective because #132525 already merged. On
release/11.0-rc1before that fix,packedsimd_alias_methodswas mis-sorted, and sincelookup_intrins()binary-searches the table,Store,StoreUnsafe,SubtractandSubtractSaturatewere all unreachable. Simulatingmono_binary_searchagainst the pre-#132525 table confirms this — so #132502 on its own would have been a silent no-op.With #132525 merged, the resulting table is strictly sorted and all 49 entries resolve;
StoreandStoreUnsafeland at indices 29 and 30.Original change
PackedSimd.Store(T* address, Vector128<T> source)takes its operands in the opposite order fromVector128.Store(this Vector128<T> source, T* destination), and the alias path only renames the method —emit_common_simd_epilogueassigns sregs in signature order. This adds them to the alias table and swaps the two sregs after the epilogue, guarded on the shape actually being a store (two parameters, void return, raw address as the second parameter). The three-argumentStoreUnsafe(source, destination, elementOffset)has no PackedSimd counterpart and still falls back to managed code.interp_packedsimd_storealso switches from assigning through av128_t*towasm_v128_store, which does not claim 16-byte alignment that neither API guarantees.Upstream measured ~6.2x on
Vector128.Store<int>and ~6.3x onVector128.StoreUnsafe<int>under the interpreter.Local validation
./build.sh clr+libs -rc release— 0 errors, 0 warningsSystem.Runtime.Intrinsics.Tests— 13011/13011 passing, 0 failed, including the three new tests:Vector128StoreUnalignedTest,Vector128StoreUnsafeUnalignedTest,Vector128StoreUnsafeElementOffsetUnalignedTestThe count is 13011 rather than upstream's 13024 because the #132499 and #132496 tests are not on this branch.
Not validated locally: the
interp-simd.cchange is wasm-only, so a native osx-arm64 build does not exercise it. It is byte-identical to the upstream commit, which passed full CI.Note
This pull request was authored with the assistance of GitHub Copilot.