Uh oh!
There was an error while loading. Please reload this page.
[mono][wasm] Intrinsify Vector128.MultiplyAddEstimate in the interpreter - #132496
Merged
Conversation
Add direct interpreter and jiterpreter lowering for Vector128<float>.MultiplyAddEstimate, using relaxed SIMD when available and separate multiply/add operations otherwise. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1025435c-e292-46cd-833f-0df02b7eb362
lewing
requested review from
BrzVlad, kotlarmilos, pavelsavara, steveisok and vitek-karas
as code ownersAugust 18, 2026 23:57
|
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. |
Contributor
Tagging subscribers to this area: @vitek-karas, @BrzVlad, @kotlarmilos |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves Mono interpreter and Browser/WASM jiterpreter handling of Vector128<float>.MultiplyAddEstimate by recognizing it as a direct SIMD intrinsic, avoiding the managed Vector64 fallback path and enabling emission of the most appropriate Wasm SIMD sequence (relaxed madd when available, otherwise mul + add).
Changes:
- Added
MultiplyAddEstimateto the recognizedVector128SIMD method set and mapped it to a new interpreter intrinsic forfloat. - Implemented the interpreter-side intrinsic for
Vector128<float>multiply-add-estimate. - Emitted
f32x4.relaxed_maddin the jiterpreter when relaxed SIMD is available, with af32x4.mul+f32x4.addfallback otherwise.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/mono/mono/mini/interp/transform-simd.c | Recognizes Vector128<float>.MultiplyAddEstimate and emits the corresponding interpreter SIMD intrinsic. |
| src/mono/mono/mini/interp/simd-methods.def | Adds MultiplyAddEstimate to the SIMD method name set. |
| src/mono/mono/mini/interp/interp-simd.c | Adds the interpreter implementation for Vector128<float> multiply-add-estimate. |
| src/mono/mono/mini/interp/interp-simd-intrins.def | Registers the new interpreter SIMD intrinsic (and feeds jiterpreter enum generation). |
| src/mono/browser/runtime/jiterpreter-trace-generator.ts | Lowers the new intrinsic to f32x4.relaxed_madd (or mul + add) in Wasm codegen. |
Uh oh!
There was an error while loading. Please reload this page.
BrzVlad
approved these changes
Aug 19, 2026
Uh oh!
There was an error while loading. Please reload this page.
lewing
commented
Aug 19, 2026
MemberAuthor
/backport to release/11.0-rc1 |
Contributor
Started backporting to |
Merged
4 tasks
lewing added a commit
that referenced
this pull request
Aug 20, 2026
…ate in the interpreter (#132524) Backport of #132496 to release/11.0-rc1 /cc @lewing ## Customer Impact - [ ] Customer reported - [ ] Found internally [Select one or both of the boxes. Describe how this issue impacts customers, citing the expected and actual behaviors and scope of the issue. If customer-reported, provide the issue number.] ## Regression - [x] Yes - [ ] No [If yes, specify when the regression was introduced. Provide the PR or commit if known.] ## Testing [How was the fix verified? How was the issue missed previously? What tests were added?] ## Risk [High/Medium/Low. Justify the indication by mentioning how risks were measured and addressed.] **IMPORTANT**: If this backport is for a servicing release, please verify that: - For .NET 8 and .NET 9: The PR target branch is `release/X.0-staging`, not `release/X.0`. - For .NET 10+: The PR target branch is `release/X.0` (no `-staging` suffix). ## Package authoring no longer needed in .NET 9 **IMPORTANT**: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version. Keep in mind that we still need package authoring in .NET 8 and older versions. Co-authored-by: Larry Ewing <lewing@microsoft.com>
lewing added a commit
that referenced
this pull request
Aug 20, 2026
…afe on wasm (#132575) 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, #132525 (backport of #132500). Once that merged, this file auto-merged with no hand edits. - **`Vector128Tests.cs`** — a pure placement conflict. #132502's tests were appended after `Vector128GetElementVariableOutOfRangeTest`, which comes from #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 #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-rc1` before that fix, `packedsimd_alias_methods` was mis-sorted, and since `lookup_intrins()` binary-searches the table, `Store`, `StoreUnsafe`, `Subtract` and `SubtractSaturate` were all unreachable. Simulating `mono_binary_search` against 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; `Store` and `StoreUnsafe` land at indices 29 and 30. ## Original change `PackedSimd.Store(T* address, Vector128<T> source)` takes its operands in the opposite order from `Vector128.Store(this Vector128<T> source, T* destination)`, and the alias path only renames the method — `emit_common_simd_epilogue` assigns 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-argument `StoreUnsafe(source, destination, elementOffset)` has no PackedSimd counterpart and still falls back to managed code. `interp_packedsimd_store` also switches from assigning through a `v128_t*` to `wasm_v128_store`, which does not claim 16-byte alignment that neither API guarantees. Upstream measured ~6.2x on `Vector128.Store<int>` and ~6.3x on `Vector128.StoreUnsafe<int>` under the interpreter. ## Local validation - `./build.sh clr+libs -rc release` — 0 errors, 0 warnings - `System.Runtime.Intrinsics.Tests` — **13011/13011 passing, 0 failed**, including the three new tests: `Vector128StoreUnalignedTest`, `Vector128StoreUnsafeUnalignedTest`, `Vector128StoreUnsafeElementOffsetUnalignedTest` The 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.c` change 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. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ebb2f49-c9fc-4c28-9755-c51b9deac735
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.
Summary
Vector128<float>.MultiplyAddEstimatedirectly in the Mono interpreterVector64fallbackf32x4.relaxed_maddfrom the Jiterpreter when relaxed SIMD is available, withf32x4.mul+f32x4.addotherwiseThis targets a major contributor to the Browser/WASM
System.Numericsregressions tracked by dotnet/perf-autofiling-issues#76371. The Numerics refactors in #130192 and #130274 introduced manyVector128.MultiplyAddEstimatecalls, but the Mono interpreter did not previously recognize that API.Performance
BenchmarkDotNet on Mono Browser/WASM, V8 15.1.182, macOS arm64 host:
Perf_Matrix3x2.MultiplyByMatrixBenchmarkPerf_Matrix4x4.InvertBenchmarkPerf_Plane.TransformByMatrix4x4BenchmarkValidation
System.Runtime.IntrinsicsBrowser/WASM tests: 13,021 passed, 0 failedNote
This pull request description was generated with GitHub Copilot.