Skip to content

[wasm] Implement RelaxedSimd intrinsics api - #130050

Open
lewing wants to merge 40 commits into
dotnet:mainfrom
lewing:wasm-relaxed-simd-api
Open

[wasm] Implement RelaxedSimd intrinsics api#130050
lewing wants to merge 40 commits into
dotnet:mainfrom
lewing:wasm-relaxed-simd-api

Conversation

@lewing

@lewing lewing commented Jun 30, 2026

Copy link
Copy Markdown
Member

Implementation of API approved in #130223

lewing and others added 6 commits June 25, 2026 17:37
Adds System.Runtime.Intrinsics.Wasm.RelaxedSimd as a sibling class
to PackedSimd, exposing the 19 opcodes from the WebAssembly Relaxed
SIMD proposal (Phase 5; shipped in Chrome 114+, Firefox 120+, V8,
and Node 22+).

Class shape mirrors the AVX10 precedent (sibling, not subclass) for
two reasons:
 1. Method-name overlap with PackedSimd on Swizzle/Min/Max has
    intentionally different semantics (relaxed = implementation-
    defined for out-of-domain inputs, deterministic on PackedSimd).
    Inheritance would silently shadow the deterministic operation.
 2. The relaxed-SIMD proposal is its own feature flag at the engine
    level, so a distinct IsSupported makes the gate explicit.

Method naming follows the PackedSimd idiom (MultiplyAdd not Fma,
ConvertToInt32 not TruncateToInt32, DotProduct to disambiguate from
PackedSimd.Dot which is the standard signed-i16 path).

The relaxed dot-product overloads take Vector128<byte> by
Vector128<sbyte> directly, encoding the spec's i7 constraint at
the type-system level.

This commit only adds the managed API surface (RelaxedSimd.cs,
RelaxedSimd.PlatformNotSupported.cs, ref source, tests). Runtime
wiring (Mono SIMD intrinsic recognizer, LLVM/AOT codegen,
WasmEnableRelaxedSimd MSBuild property, IsSupported runtime
detection) follows in stacked commits.

API proposal draft: docs/ at ~/.copilot/session-state/.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Match Vector128.MultiplyAddEstimate's naming for the 'may or may not
be true FMA' semantic. The Wasm relaxed_madd / relaxed_nmadd ops are
exactly that: the runtime may or may not emit a true fused multiply
add depending on the host. FusedMultiplyAdd remains reserved for
guaranteed-fusion (Avx512F, AdvSimd.Fma, etc.).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Introduces the build-time switch for the Wasm Relaxed SIMD extension
(API surface added in the previous commit). When the property is set
to true, the build:

 - threads -mrelaxed-simd to emcc (BrowserWasmApp.targets)
 - threads --enable-relaxed-simd to wasm-opt (WasmApp.Common.targets)
 - threads mattr=+relaxed-simd to the AOT compiler (likewise)
 - sets WASM_ENABLE_RELAXED_SIMD=1 in the emscripten env
 - includes ILLink.Substitutions.WasmRelaxedSimd.xml so that
   RelaxedSimd.get_IsSupported is stubbed to true by the trimmer

When the property is false (default), the parallel
ILLink.Substitutions.NoWasmRelaxedSimd.xml stubs IsSupported to
false. The test infra in eng/testing/tests.wasm.targets mirrors the
substitution wiring for the BuildAOTTestsOnHelix=true path that
sidesteps BrowserWasmApp.targets.

Validation that WasmEnableRelaxedSimd=true requires
WasmEnableSIMD=true is performed at build time (in the
_WasmCommonPrepareForWasmBuildNative target) with a clear error
message.

Default is false everywhere. The browser-wasm flavor will likely
want to flip to true once the runtime wiring (Mono SIMD intrinsic
recognizer + LLVM intrinsic table + interpreter handlers) lands in
subsequent commits.

Validated locally with both ./build.sh mono+libs -os browser -c
Release (default off) and the same with /p:WasmEnableSIMD=true
/p:WasmEnableRelaxedSimd=true. Both succeed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds 13 INTRINS_* entries for the WebAssembly relaxed-SIMD opcodes,
mapping to LLVM's Intrinsic::wasm_relaxed_* function declarations:

 - relaxed swizzle (i8x16)
 - relaxed truncating float-to-int (f32x4 and f64x2->i32x4)
 - relaxed multiply-add / negated multiply-add (f32x4, f64x2)
 - relaxed lane select (i8x16, i16x8, i32x4, i64x2)
 - relaxed min / max (f32x4, f64x2)
 - relaxed q15mulr signed
 - relaxed dot i8x16 i7x16 signed
 - relaxed dot i8x16 i7x16 add signed

Non-overloaded ops use INTRINS(...) (single fixed signature in LLVM);
overloaded ones (madd, nmadd, laneselect, min, max) use
INTRINS_OVR_TAG(...) with element-width tag bitmasks so the existing
overloaded-intrinsic registration path picks the correct concrete
type at call time.

This commit is no-op on its own: nothing references INTRINS_WASM_
RELAXED_* yet. The Mono SIMD intrinsic recognizer wiring in the
next commit will dispatch RelaxedSimd.* methods to these IDs.

Build verified: ./build.sh mono+libs -os browser -c Release passes
with mono-aot-cross successfully linked.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pieces 2 + 3 of the relaxed-SIMD support: declares the
MONO_CPU_WASM_RELAXED_SIMD CPU feature, recognizes the
'+relaxed-simd' mattr passed by the AOT compiler, advertises the
feature to the LLVM CPU-features query, and dispatches the
RelaxedSimd.* managed methods to the LLVM intrinsics added in the
previous commit.

Specifics:
 - mini.h: add MONO_CPU_WASM_RELAXED_SIMD = 1 << 3.
 - aot-compiler.c: parse 'relaxed-simd' (with optional +/- prefix)
   in mattr arguments.
 - mini-llvm.c flags_map: advertise the feature so the runtime
   reports it when the AOT'd code is loaded.
 - simd-methods.h: register the four new method names not previously
   used elsewhere — DotProductAdd, LaneSelect,
   MultiplyAddNegatedEstimate, MultiplyRoundedQ15. (DotProduct,
   MultiplyAddEstimate, Swizzle, Min, Max, ConvertToInt32,
   ConvertToUInt32 reuse existing entries.)
 - simd-intrinsics.c: new relaxedsimd_methods[] table mapping each
   method to its (opcode, intrinsic-id) pair; register the group
   under 'RelaxedSimd'/MONO_CPU_WASM_RELAXED_SIMD; add an
   emit-wasm-supported-intrinsics block to dispatch the type-
   discriminated ConvertToInt32/UInt32 overloads (float vs double).
 - mini-llvm.c: hoist case OP_XOP_X_X_X_X out of the TARGET_ARM64
   block (DotProductAdd is the first non-arm consumer); keep the
   INTRINS_AARCH64_SHA1{C,M,P} bits under TARGET_ARM64.

End-to-end validation on browser-wasm AOT (Chromium 143,
WasmEnableSIMD=true, WasmEnableRelaxedSimd=true):
System.Runtime.Intrinsics.Tests 13023/13023 passing including the
six new RelaxedSimdTests (DotProduct, DotProductAdd,
MultiplyAddEstimate, LaneSelect, Swizzle, IsSupportedReflects).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…library variant

Mirrors the existing PackedSimd library structure: the SDK ships three static
mono-wasm interp-simd library variants and the app build picks one based on its
SIMD selection (the existing WasmEnableSIMD switch plus the new WasmEnableRelaxedSimd):

  libmono-wasm-nosimd.a       (default)         WasmEnableSIMD=false
  libmono-wasm-simd.a         -msimd128         WasmEnableSIMD=true                       (default when SIMD on)
  libmono-wasm-relaxed-simd.a -msimd128 -mrelaxed-simd  WasmEnableSIMD=true, WasmEnableRelaxedSimd=true

This avoids forcing a runtime-build-time choice (the previous
MonoWasmEnableRelaxedSimd flag, now removed) and keeps WasmEnableRelaxedSimd a
pure per-app property paralleling the rest of the relaxed-simd wiring (AOT mattr,
emcc -mrelaxed-simd, wasm-opt --enable-relaxed-simd, ILLink IsSupported stub).

How the .def routes the relaxed-simd entries:

* New INTERP_WASM_RELAXED_SIMD_INTRINSIC_V_{V,VV,VVV} macro family is set up in
  the prelude of interp-simd-intrins.def. With HOST_WASM_RELAXED_SIMD defined it
  aliases the regular INTERP_WASM_SIMD_INTRINSIC_V_{V,VV,VVV} macros, so the
  relaxed-simd library generates real emscripten intrinsic wrappers and tables
  with the real Wasm opcode values (0x100-0x113). Without the define the macros
  route the table entries to per-arity stub functions
  (_mono_interp_simd_relaxed_unsupported_{1,2,3}) and clear the Wasm-opcode slot,
  so libmono-wasm-simd.a still exports all the same symbols with stable indices
  and the jiterpreter falls back to the C helper (which asserts but should be
  unreachable because RelaxedSimd.IsSupported reports false).
* transform-simd.c (in mono-ee-interp) compiles the relaxed entries
  unconditionally through the same INTRINS_COMMON path that PackedSimd uses, so
  the lookup table and MintSIMDOpsPP/PPP/PPPP enums in mintops.h have identical
  layouts across both library flavors. A single mono-ee-interp.a links cleanly
  against either variant.
* genmintops.py learns the new RELAXED macro names so the TypeScript SimdIntrinsic
  enums and SimdInfo[] stay in sync.
* emit_sri_relaxedsimd in transform-simd.c keys RelaxedSimd.IsSupported and the
  intrinsic dispatch off mono_interp_relaxed_simd_supported, an extern int
  exported by both library variants (1 in relaxed, 0 in regular).
* Method-name collisions with PackedSimd (Swizzle, Min, Max, MultiplyAddEstimate,
  ConvertToInt32, ConvertToUInt32) are sidestepped by prefixing the .def entry
  names with 'Relaxed'; emit_sri_relaxedsimd prepends 'Relaxed' before lookup.
  The public API surface keeps the unprefixed names.
* installer manifest learns the new libmono-wasm-relaxed-simd.a entry.

Jiterpreter wiring is fully data-driven and needs no TypeScript changes: the
existing emit_simd_3/emit_simd_4 fast path calls mono_jiterp_get_simd_opcode
which returns the relaxed-simd Wasm opcode value, and appendSimd encodes it
through appendULeb so the 2-byte LEB128 form for opcodes >= 0x80 is handled
correctly.

Validated on browser-wasm Release with Chromium 143:
* AOT, WasmEnableRelaxedSimd off: System.Runtime.Intrinsics.Tests
  RelaxedSimdTests = 1 passed + 5 ConditionalFact skipped.
* AOT, WasmEnableRelaxedSimd on: full System.Runtime.Intrinsics suite
  13023/13023 pass.
* Interp-only, both off and on: 1 passed + 5 skipped (interp path uses the
  SDK's pre-built dotnet.native.wasm, which currently always links
  libmono-wasm-simd.a; AOT is required to exercise the relaxed-simd helpers
  end-to-end. Shipping a second dotnet.native.wasm variant for interp-only
  apps is a future enhancement.)
Copilot AI lite review requested due to automatic review settings June 30, 2026 18:42
@dotnet-policy-service dotnet-policy-service Bot added the linkable-framework Issues associated with delivering a linker friendly framework label Jun 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 25 out of 25 changed files in this pull request and generated 4 comments.

Comment thread src/libraries/System.Runtime.Intrinsics/ref/System.Runtime.Intrinsics.cs Outdated
Comment thread src/mono/browser/browser.proj Outdated
lewing and others added 2 commits June 30, 2026 14:55
…n nosimd lib; wire WasiApp.targets

Two CI regressions exposed on PR dotnet#130050:

1. browser.proj was still passing -DENABLE_WASM_RELAXED_SIMD=0 to the dotnet.native
   cmake configure based on a removed MonoWasmEnableRelaxedSimd property. Nothing
   under src/mono/browser/runtime/CMakeLists.txt consumes that variable, so CMake's
   'Manually-specified variables were not used' diagnostic was elevated to a build
   error across all browser-wasm Build legs. Drop the dead line.

2. WASI's prebuilt dotnet.wasm always links libmono-wasm-nosimd.a (the runtime cmake
   never receives WasmEnableSIMD=true), but the new mono_interp_relaxed_simd_supported
   extern is referenced unconditionally from libmono-ee-interp.a under HOST_BROWSER ||
   HOST_WASI. Definitions only lived in libmono-wasm-{simd,relaxed-simd}.a, leaving
   the WASI link with an undefined symbol. Provide a default '= 0' definition in
   interp-nosimd.c so every SIMD-library variant exports the symbol.

Also mirror the browser app-build library selection into WasiApp.targets so WASI
apps can opt into the relaxed-simd library variant when WasmEnableRelaxedSimd=true,
matching the BrowserWasmApp.targets behavior.

Validated locally:
* browser-wasm Release mono+libs build clean.
* wasi-wasm Release mono+libs build clean.
* AOT smoke: System.Runtime.Intrinsics.Tests with WasmEnableSIMD=true and
  WasmEnableRelaxedSimd=true: 13023/13023 pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The PR's third SIMD library variant (libmono-wasm-relaxed-simd.a) ships in the
runtime pack but is picked up by the broad '*.a' glob in {Browser,Wasi}App.targets
that scoops every static lib into _WasmNativeFileForLinking. The selection logic
sets _WasmSIMDLibToExclude to a semicolon-separated list of bare filenames and
then does:

    <_WasmNativeFileForLinking
        Remove="$(MicrosoftNetCoreAppRuntimePackRidNativeDir)$(_WasmSIMDLibToExclude)" />

MSBuild splits the semicolons into items but only prefixes the FIRST one with the
runtime-pack directory; subsequent items become bare filenames that don't match
the full-path entries in _WasmNativeFileForLinking, so the second exclusion is
silently dropped and libmono-wasm-relaxed-simd.a leaks into every test app's link
line. The post-link wasm-opt then fails because it isn't invoked with
--enable-relaxed-simd:

    [wasm-validator error in function _mono_interp_simd_wasm_i32x4_relaxed_trunc_f32x4]
      unexpected false: all used features should be allowed, on
      (i32x4.relaxed_trunc_f32x4_s ...)
    [--enable-relaxed-simd]
    Fatal: error validating input

Fix: embed the runtime-pack directory prefix into every value of
_WasmSIMDLibToExclude so the Remove attribute receives fully-qualified paths
that actually match the items added by the glob. Applied symmetrically to
BrowserWasmApp.targets and WasiApp.targets.

Validated locally on browser-wasm:
* Default (WasmEnableRelaxedSimd unset): System.Runtime.Intrinsics.Tests
  RelaxedSimdTests = 1 passed + 5 ConditionalFact-skipped (was failing
  wasm-opt validation pre-fix), full suite 13018 passed + 5 skipped.
* WasmEnableRelaxedSimd=true: full suite 13023/13023 pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 30, 2026 21:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 26 out of 26 changed files in this pull request and generated 3 comments.

Comment thread src/mono/mono/mini/interp/transform-simd.c
Comment thread src/libraries/System.Runtime.Intrinsics/ref/System.Runtime.Intrinsics.cs Outdated
@lewing lewing changed the title Wasm relaxed simd api Wasm RelaxedSimd intrinsics experiment Jul 1, 2026
@lewing

lewing commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

@tannergooding relaxedsimd is standardized now, there is no rush but it would be nice to figure out the api details

@tannergooding

Copy link
Copy Markdown
Member

We should just need to get an API proposal up and it will be relatively straightforward to get in/approved.

The surface in here looks generally correct at a glance, I'd just need to go through the full proposal against the finalized spec before marking it ready-for-review

@lewing

lewing commented Jul 5, 2026

Copy link
Copy Markdown
Member Author

API proposal opened at #130223 (tracks the surface added in this PR). Marking this PR as dependent on the API-review outcome there.

Note

This comment was created with the assistance of GitHub Copilot.

- Correct DotProduct/DotProductAdd operand types per the finished WebAssembly
  relaxed-simd spec pseudocode: operand `a` is signed and operand `b` is
  unsigned-7-bit, not the reversed (byte, sbyte) shape the prototype had.
  Updated the impl, PlatformNotSupported, ref, and tests.

- Fix RelaxedSimd IsSupported reflection test: assert MethodInfo is non-null
  before invoking, so a lookup miss yields a diagnostic instead of NRE.

- Replace float.Epsilon-based ULP-style tolerance in the MultiplyAddEstimate
  test with an explicit 1e-5 relative tolerance. float.Epsilon is the smallest
  subnormal (~1.4e-45), not the unit roundoff, so the previous check was
  effectively exact-equality.

- Document the reflection-reach stack-overflow invariant in the Mono interp
  emit_sri_relaxedsimd path so future editors preserve the IsSupported gate
  contract, and note the general follow-up covering all wasm intrinsic classes.

Addresses copilot-pull-request-reviewer feedback on dotnet#130050 and
tracks the API-review corrections captured in dotnet#130223.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lewing

lewing commented Jul 5, 2026

Copy link
Copy Markdown
Member Author

Pushed 7f99067 addressing the Copilot review feedback plus a spec-derived correction to DotProduct/DotProductAdd:

  • DotProduct operand types corrected: signatures are now (Vector128<sbyte> left, Vector128<byte> right) per the finished spec pseudocode (a is signed, b is unsigned-7-bit). The prototype had these reversed.
  • RelaxedSimdIsSupportedReflects: now asserts MethodInfo is non-null before invoking, so a lookup miss reports a diagnostic instead of throwing NRE.
  • MultiplyAddFloatMatchesScalarApproximately: replaced the float.Epsilon-scaled tolerance (which was subnormal-scaled, effectively exact-equality) with an explicit 1e-5 relative tolerance.
  • Interp fallback documentation: added a comment in emit_sri_relaxedsimd documenting the reflection-reach stack-overflow invariant and the fact that this is the same general risk pattern shared by every wasm intrinsic class (PackedSimd, WasmBase). A uniform "throw PNSE from unmatched stubs" fix belongs in a separate change covering all wasm intrinsic classes.
  • Dead MonoWasmEnableRelaxedSimd MSBuild plumbing: verified already removed by the earlier CI-fix commits (ff5d1cff65c / bfebb64b704); no code change needed.

API-review issue #130223 updated with the corrected surface and prototype pointer.

Note

This comment was created with the assistance of GitHub Copilot.

using System.Runtime.Intrinsics.Wasm;
using Xunit;

namespace System.Runtime.Intrinsics.Wasm.Tests

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should the RelaxedSimdTests be under src\tests\JIT\HardwareIntrinsics\Wasm to match how hardware intrinsics are tested on other architectures?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The required steps for mono aren't run there but we can do both and remove this location when it is no longer needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added matching RelaxedSimd coverage under src/tests/JIT/HardwareIntrinsics/Wasm, with ReadyToRun and WasmEnableRelaxedSimd=true; the dedicated Wasm hardware-intrinsics pipeline now requests that instruction set. The library copy remains for Mono browser execution, matching the current PackedSimd split. Both new JIT projects compile, and the library suite now also runs on CoreCLR instead of being skipped.

Note

This reply was generated with GitHub Copilot.

Note

Auto-replied by the GitHub Copilot app.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The required steps for mono aren't run there but we can do both and remove this location when it is no longer needed?

Why are we spending time on adding this to Mono in the first place? It will be deleted in several months. The API implementation can be stubbed out as unsupported for now to keep Mono functional.

@lewing lewing Sep 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

because it is already done and can establish a better performance baseline right now. the mono side was working months ago.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don’t find sunk-cost justifications compelling. This is causing us to unnecessary throwaway work: the tests aren’t aligned with the other HW intrinsic tests that will need cleaning up later (as discussed here), and we are spending time fixing bugs in the Mono implementations (e.g. 647195b).

@lewing lewing Sep 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Packedsimd will need cleanup already. With mono I can build use app code with relaxedsimd after this lands, coreclr isn't there yet. I'm happy to remove the tests now if you want but I own both sides of the mono cleanup so I'm not sure what your objection is?

wrt to the opcodes in 647195b I believe those the draft versions but I would have to check.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With mono I can build use app code with relaxedsimd after this lands, coreclr isn't there yet.

What's missing so you can do your tests on coreclr instead? Can we wait for that to happen?

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0e443132-2985-4b7b-8c59-28498d381fa0
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0e443132-2985-4b7b-8c59-28498d381fa0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Unresolved critical, moderate, and nit findings remain in runtime wiring, build selection, test coverage, and test configuration.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 1 High severity · 1 Medium severity

Open findings (2)
Resolved findings (1)

Comment thread src/mono/wasm/Wasm.Build.Tests/WasmSIMDTests.cs
Comment thread src/mono/browser/browser.proj
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0e443132-2985-4b7b-8c59-28498d381fa0
@lewing

lewing commented Sep 12, 2026

Copy link
Copy Markdown
Member Author

lib-free detection is:

const hasFixedWidthSimd = WebAssembly.validate(new Uint8Array([
0, 97, 115, 109, // \0asm
1, 0, 0, 0, // Version 1
1, 5, 1, 96, 0, 1, 123, // Type section (returns v128)
3, 2, 1, 0, // Function section
10, 10, 1, 8, 0, // Code section header
65, 0, // i32.const
253, 15, // i32x4.splat
253, 98, // i8x16.shuffle
11 // end
]));

// true on current versions of major browsers
const hasRelaxedSimd = WebAssembly.validate(new Uint8Array([
0, 97, 115, 109, // \0asm
1, 0, 0, 0, // Version 1
1, 5, 1, 96, 0, 1, 123, // Type section (returns v128)
3, 2, 1, 0, // Function section
10, 15, 1, 13, 0, // Code section header
65, 1, // i32.const
253, 15, // i32x4.splat
65, 2, // i32.const
253, 15, // i32x4.splat
253, 128, 2, // i8x16.relaxed_swizzle
11 // end
]));

// false on current Safari version, otherwise true in other major browsers
const binaryToFetch = hasRelaxedSimd ? "app.relaxed.wasm" : hasFixedWidthSimd ?
"app.fixed.wasm" : "app.baseline.wasm";

yeah, and there are libraries like https://github.com/GoogleChromeLabs/wasm-feature-detect that make it very easy to do the detection but they don't solve the multiple build requirement. With the jitperpreter in mono it was possible to recognize interpreter intrinsics in directly and dynamically replace them with the real opcode if supported. Both of those paths are interesting but out of scope for the current coreclr standup effort.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Critical relaxed-SIMD opcode mismatches and browser/test configuration issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 2 High severity

Open findings (2)
Resolved findings (2)
Previously missed findings (1)

In code that hasn't changed since last review

Medium severity Enable regular SIMD in the positive relaxed-SIMD test

src/​mono/​wasm/​Wasm.Build.Tests/​WasmSIMDTests.cs:85

The true test case passes isNativeBuild: true, but this property string sets only WasmEnableRelaxedSimd. WasmApp.Common.targets now rejects native builds when relaxed SIMD is enabled without WasmEnableSIMD=true, so the positive Mono case fails before it can run. Include the regular SIMD property in this test setup.

Comment thread src/coreclr/jit/instrswasm.h
Comment thread src/mono/mono/mini/interp/interp-simd-intrins.def
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0e443132-2985-4b7b-8c59-28498d381fa0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note

Copilot was unable to run its full agentic suite in this review.

Copilot review overview

Review tier: Lite
Findings: 1 Medium severity

Open findings (1)
Resolved findings (2)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note

Copilot was unable to run its full agentic suite in this review.

Copilot review overview

Review tier: Lite
Findings: 1 High severity · 1 Medium severity

Open findings (2)

@lewing

lewing commented Sep 12, 2026

Copy link
Copy Markdown
Member Author

the perf pipeline failure is unrelated but the fix is dotnet/performance#5310

@lewing

lewing commented Sep 12, 2026

Copy link
Copy Markdown
Member Author

/azp run runtime-wasp-perf

@azure-pipelines

Copy link
Copy Markdown
No pipelines are associated with this pull request.

@lewing

lewing commented Sep 12, 2026

Copy link
Copy Markdown
Member Author

/azp run runtime-wasm-perf

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Comment thread src/libraries/System.Runtime.Intrinsics/tests/Wasm/RelaxedSimdTests.cs Outdated
Comment thread src/coreclr/crossgen-corelib.proj Outdated
Make the Wasm CoreLib Crossgen2 capability unconditional while keeping per-app RelaxedSimd compilation opt-in. Remove redundant trimming annotations and unsafe modifiers from the tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0e443132-2985-4b7b-8c59-28498d381fa0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Unresolved critical crossgen and jiterpreter correctness findings remain, along with intrinsic-coverage and test-configuration gaps.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 2 High severity

Open findings (2)
Resolved findings (2)

Comment thread src/coreclr/crossgen-corelib.proj Outdated
Comment thread src/mono/browser/build/BrowserWasmApp.targets
Crossgen2 already understands RelaxedSimd. Only opt-in app publishing should target the relaxed SIMD instruction set; the shared runtime-pack CoreLib must remain baseline-compatible.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0e443132-2985-4b7b-8c59-28498d381fa0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 3 High severity

Open findings (3)
Resolved findings (1)
Previously missed findings (3)

In code that hasn't changed since last review

Medium severity Keep the regular test project unoptimized

src/​tests/​JIT/​HardwareIntrinsics/​Wasm/​RelaxedSimd/​RelaxedSimdTests_r.csproj:5

This is opposite to the sibling PackedSimdTests_r.csproj, where the regular _r project uses <Optimize /> and the _ro project is optimized. Swap this value so the new projects cover the same regular-versus-optimized configurations as the existing Wasm intrinsic tests.

Medium severity Match the optimized test project convention

src/​tests/​JIT/​HardwareIntrinsics/​Wasm/​RelaxedSimd/​RelaxedSimdTests_ro.csproj:5

This reverses the established Wasm intrinsic test configuration: PackedSimdTests_ro.csproj uses <Optimize>True</Optimize> while PackedSimdTests_r.csproj uses <Optimize />. With the new settings, the _ro project is built unoptimized and the _r project optimized, so these tests no longer exercise the intended R2R variants.

Low severity Correct the mono-ee-interp build description

src/​mono/​mono/​mini/​interp/​interp-simd-intrins.def:53

The comment says mono-ee-interp is always compiled with HOST_WASM_RELAXED_SIMD=1, but the CMake definition is applied only to mono-wasm-relaxed-simd; mono-ee-interp is created from ${interp_sources} without that definition. Please describe the fallback macros as preserving stable table entries instead of claiming the generic interpreter library is built with the relaxed-SIMD define.

Comment thread src/mono/browser/browser.proj
Comment thread src/mono/browser/runtime/startup.ts
Mirror the in-tree CoreCLR browser app wiring in Microsoft.NET.Sdk.WebAssembly.Pack so opt-in publishes pass the RelaxedSimd instruction set to Crossgen2 and advertise support through runtime configuration.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0e443132-2985-4b7b-8c59-28498d381fa0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pass --enable-relaxed-simd to the Release runtime-pack wasm-opt step whenever the prelinked runtime is built with WasmEnableRelaxedSimd=true.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0e443132-2985-4b7b-8c59-28498d381fa0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note

Copilot was unable to run its full agentic suite in this review.

Copilot review overview

Review tier: Lite
Findings: 15 High severity · 2 Medium severity

Open findings (17)
Resolved findings (4)

/// <summary>Gets a value that indicates whether the APIs in this class are supported.</summary>
/// <value><see langword="true" /> if the APIs are supported; otherwise, <see langword="false" />.</value>
/// <remarks>A value of <see langword="false" /> indicates that the APIs will throw <see cref="PlatformNotSupportedException" />.</remarks>
public static bool IsSupported { get { return IsSupported; } }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-wasm WebAssembly architecture area-System.Runtime.Intrinsics linkable-framework Issues associated with delivering a linker friendly framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants