Materialize Vector128<T> as a wasm v128 in the codegen ABI

[tannergooding]

Today several wasm codegen paths bail to the interpreter (NYI_WASM_SIMD) because a 128-bit SIMD value arrives as an i32 (a by-ref pointer) rather than a real v128 on the wasm value stack. Emitting a v128.* op against an i32 operand produces an invalid module, so the JIT defensively bails. This is the end-to-end blocker for SIMD actually executing on wasm.

This change treats Vector128<T> as the wasm v128 ABI primitive and materializes it by value:

• getWasmLowering maps Vector128<T> to CORINFO_WASM_TYPE_V128, and ToJitType maps that to TYP_SIMD16.
• The five codegen sites that previously bailed now emit real v128 values: SIMD parameter homing, SIMD16 local field load, store-indirect, call argument, and local load.
• RaiseSignature can round-trip the 'V' signature char back to a concrete v128 type.

---

Why only Vector128<T>. It is the single SIMD type the JIT recognizes as TYP_SIMD16 on wasm. In getBaseTypeAndSizeOfSIMDType, Vector128<T> (16 bytes) is not target-gated, where-as Vector64<T> is TARGET_ARM64-only and Vector256<T>/Vector512<T> are TARGET_XARCH-only, so on wasm those fall through to TYP_UNDEF and are handled as regular structs. The System.Numerics vectors (Vector2/Vector3/Vector4, Vector<T>, Plane, Quaternion) are multi-field, so getWasmLowering classifies them as passed by reference (they arrive as TYP_BYREF, not varTypeIsSIMD). The net effect is that the only by-value SIMD value reaching these codegen paths is a Vector128<T> in a real v128 register, so the previous bails are no longer needed and no new gate is required. The other SIMD types keep their pre-change ABI and are deferred to follow-ups.

Because every 'V' in a lowered signature is now exactly 16 bytes, there is no mixed-width offset ambiguity in the signature round-trip.

---

Testing. Adds an R2R test (WasmSimdModule) exercising the v128 calling convention — parameter, return, through-local, store-indirect, and call-argument — using Vector128<int> without any SIMD arithmetic intrinsics, so only the ABI/materialization paths are covered. Locally, ILCompiler.ReadyToRun.Tests (filter Wasm) passes 2/2; wasm SIMD only executes in CI, which is the point of this draft.

Opening as draft for full CI validation.

Note

This PR description was drafted with GitHub Copilot.

[azure-pipelines]

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

[Copilot]

Pull request overview

Updates the wasm ABI lowering and wasm JIT codegen so Vector128<T> is treated as a real wasm v128 value (instead of arriving as an i32 byref), enabling SIMD values to flow through key codegen paths without bailing to interpreter.

Changes:

• Teach wasm lowering/signature logic to classify Vector128<T> as v128 and round-trip 'V' signature encodings.
• Update wasm JIT ABI/type mapping and remove several NYI_WASM_SIMD bailouts now that v128 is materialized by-value.
• Add an R2R wasm test module and a new xUnit test suite entry to exercise the v128 calling convention plumbing.

Show a summary per file

File	Description
src/coreclr/tools/Common/JitInterface/WasmLowering.cs	Recognize Vector128<T> as v128, emit 'V' in signature strings, and enable signature round-tripping.
src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs	Plumbs WasmValueType.V128 into the JIT-facing CorInfoWasmType lowering.
src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.Wasm.cs	Adds caching for a representative v128 type to support 'V' signature raising.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/WasmInterpreterToR2RThunkNode.cs	Adds argument materialization support for v128 in interpreter-to-R2R thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/Webcil/WasmSimdModule.cs	New test-case source to exercise v128 param/return/local/store/call-arg flows.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.cs	Adds a new test that compiles the wasm SIMD test case into a .wasm R2R artifact.
src/coreclr/jit/targetwasm.cpp	Maps CORINFO_WASM_TYPE_V128 to TYP_SIMD16 for wasm ABI classification.
src/coreclr/jit/codegenwasm.cpp	Removes several SIMD-related NYI bailouts in prolog/loads/stores/calls now that v128 is supported.

Copilot's findings

• Files reviewed: 8/8 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 8/8 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 8/8 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

• Files reviewed: 8/8 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 8/8 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 8/8 changed files
• Comments generated: 1

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "7ffe7916ffb20d3db9d718d372e6bd0fffd2e0af",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "46ec2c1bce94b5cc262049195c5b5b50a64708e9",
  "last_reviewed_commit": "7ffe7916ffb20d3db9d718d372e6bd0fffd2e0af",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "46ec2c1bce94b5cc262049195c5b5b50a64708e9",
  "last_recorded_worker_run_id": "29688140671",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "7ffe7916ffb20d3db9d718d372e6bd0fffd2e0af",
      "review_id": 4730810927
    }
  ]
}

[github-actions]

Holistic Review

Motivation: On wasm, several codegen paths currently bail to the interpreter (NYI_WASM_SIMD) because a 128-bit SIMD value arrives as an i32 by-ref pointer rather than a real v128 on the wasm value stack. Emitting a v128.* op against an i32 operand yields an invalid module, so the JIT defensively bails. This blocks SIMD from actually executing on wasm. The motivation is clear, correct, and well-justified.

Approach: The PR treats Vector128<T> as the wasm v128 ABI primitive and materializes it by value. getWasmLowering/LowerType map Vector128<T> (primitive-numeric T only) to CORINFO_WASM_TYPE_V128; WasmClassifier::ToJitType maps that to TYP_SIMD16; and the five codegen sites that previously bailed (SIMD parameter homing, SIMD16 local field load, store-indirect, call argument, and local load) are removed so real v128 values flow through. RaiseSignature round-trips the 'V' signature char via a newly cached representative v128 type. The IsPrimitiveNumeric gate in the three ARM64 ComputeValueTypeShapeCharacteristics sites is refactored into a shared VectorFieldLayoutAlgorithm.IsSupportedVectorBaseType helper, reused by the new wasm check. The scoping rationale for restricting to Vector128<T> (other SIMD types remain multi-field/by-ref or are target-gated to non-wasm) is sound and preserves existing ABI for those types.

Summary: This is a focused, correct step that removes the interpreter bails and wires Vector128<T> through the wasm v128 ABI by value. The IsSupportedVectorBaseType refactor is behavior-preserving for the existing ARM64 sites (it currently just wraps IsPrimitiveNumeric) while centralizing the definition. The 'V' round-trip via CachedV128Type is safe because every lowered 'V' is exactly a 16-byte v128, so the JIT's element-type-agnostic TYP_SIMD16 mapping makes the specific cached instantiation immaterial; the caching uses the same non-atomic ??= pattern as the existing empty-struct/struct-by-size caches. The added R2R test (WasmSimdModule) validates the ABI paths (parameter, return, through-local, store-indirect, call-argument) by asserting the emitted wasm valtype is 0x7B (v128), which is a meaningful regression guard against reverting to the by-ref i32 ABI. No arithmetic intrinsics are exercised, matching the stated scope. I found no correctness, security, or convention issues in the changed lines. Verdict: LGTM. As the author notes, this is a draft pending full CI (wasm SIMD only executes in CI), which is the appropriate gate before merge.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 65.4 AIC · ⌖ 10.6 AIC · ⊞ 10K

[Copilot]

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 0 new

[davidwrighton]

I'd like to understand the rationale behind some choices, and push back on the not handling the wrapper struct scenario.

[Copilot]

Copilot's findings

Comments suppressed due to low confidence (1)

src/coreclr/tools/Common/JitInterface/WasmLowering.cs:154

• LowerType checks IsWasmV128Type(type) before lowering, but LowerToAbiType can now lower a non-intrinsic wrapper struct (single-field wrapper around v128) to a v128 ABI type. In that case abiType becomes Vector128<T>/Vector<T> and the subsequent switch (abiType.UnderlyingType.Category) will hit the default path and throw, even though the ABI lowering succeeded. Adding a post-lowering v128 check keeps LowerType consistent with LowerToAbiType and makes it safe to call with wrapper structs.

            TypeDesc abiType = LowerToAbiType(type);

            if (abiType == null)
            {
                return pointerType;
            }

• Files reviewed: 11/11 changed files
• Comments generated: 0 new