[RyuJIT Wasm] SIMD Element-Wise Loads and Stores

[adamperlin]

This PR implements support for SIMD element wise loads and stores, including jump table support for LoadScalarAndInsert and StoreSelectedScalar.

[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.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR updates the WASM RyuJIT SIMD hardware-intrinsics pipeline to treat certain PackedSimd operations as real memory load/store intrinsics (including lane-immediate forms) and extends the existing immediate/jump-table lowering/codegen support to cover those cases.

Changes:

• Introduces a WASM-aware “has required immediate operand” flag (HW_Flag_HasImmediateOperand) and switches WASM immediate detection to use flags rather than HW_Category_IMM.
• Reclassifies several PackedSimd intrinsics (notably LoadScalarAndInsert / StoreSelectedScalar and some scalar loads) into HW_Category_MemoryLoad / HW_Category_MemoryStore with opcode mappings, enabling table-driven codegen.
• Extends WASM codegen and jump-table fallback logic to emit lane-memarg forms (emitIns_MemargLane) for element-wise lane loads/stores.

Reviewed changes

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

Show a summary per file

File	Description
src/coreclr/jit/lowerwasm.cpp	Uses HasImmediateOperand for containment/lowering decisions and routes new lane load/store intrinsics through immediate lowering.
src/coreclr/jit/hwintrinsicwasm.cpp	Removes placeholder “special import” handling for intrinsics that are now table-driven/memory-categorized.
src/coreclr/jit/hwintrinsiclistwasm.h	Reclassifies and maps PackedSimd load/store lane intrinsics to WASM SIMD opcodes and adds immediate flags.
src/coreclr/jit/hwintrinsiccodegenwasm.cpp	Adds table-driven emission for HW_Category_MemoryLoad/Store and extends jump-table fallback for lane memarg intrinsics.
src/coreclr/jit/hwintrinsic.h	Adds HW_Flag_HasImmediateOperand for WASM and updates HasImmediateOperand to use flags on WASM.
src/coreclr/jit/gentree.cpp	Enables OperIsMemoryLoad/Store classification for WASM hardware intrinsics.
src/coreclr/jit/codegenwasm.cpp	Removes the previous NYI guard for indirect SIMD16 stores.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/tests/JIT/HardwareIntrinsics/Wasm/PackedSimd/PackedSimdTests.cs:535

• Jump-table fallback for non-constant lane indices is a key part of this change (LoadScalarAndInsert / StoreSelectedScalar), but the new tests here only validate the null-check path. It would be good to add a deterministic test that actually executes these intrinsics with a non-null address and a non-constant lane index (the existing NoInlining wrappers already ensure the index is non-constant within the callee), so we validate the jump-table codegen and per-lane semantics end-to-end.

        Vector128<long> ll = Opaque(Vector128.Create(10L, 20L));
        Vector128<long> rl = Opaque(Vector128.Create(30L, 40L));
        Assert.Equal(Vector128.Create(10L, 30L), Vector128.UnzipEven(ll, rl));
        Assert.Equal(Vector128.Create(20L, 40L), Vector128.UnzipOdd(ll, rl));

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[adamperlin]

Crossgen of S.P.C is failing now, investigating...

[Copilot]

Pull request overview

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

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "67094186c371b24a19ffbfa83e11b29139651d9f",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "840cea322a2978a568e8cdfd4d2e643fcf42aa1c",
  "last_reviewed_commit": "67094186c371b24a19ffbfa83e11b29139651d9f",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "840cea322a2978a568e8cdfd4d2e643fcf42aa1c",
  "last_recorded_worker_run_id": "29766980294",
  "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": "e78535b29e75251d63cd1f53f09336c2ef90caee",
      "review_id": 4730813646
    },
    {
      "commit": "67094186c371b24a19ffbfa83e11b29139651d9f",
      "review_id": 4737830632
    }
  ]
}

[github-actions]

Holistic Review

Motivation: Extend the RyuJIT WebAssembly backend to support SIMD element-wise loads and stores, wiring up LoadScalarAndInsert, LoadScalarVector128, LoadScalarAndSplatVector128, LoadWideningVector128, and StoreSelectedScalar as real code-generating intrinsics (including jump-table fallback for LoadScalarAndInsert/StoreSelectedScalar when the lane index is not a compile-time constant). Previously these were HW_Flag_InvalidNodeId|HW_Flag_SpecialImport placeholders or NYI paths.

Approach: The change decouples "has an immediate operand" from the HW_Category_IMM category by adding a new HW_Flag_HasImmediateOperand flag for WASM (mirroring the existing ARM64 model), so HasImmediateOperand, needsJumpTableFallback, and ContainCheckHWIntrinsic key off the flag rather than the category. HW_Category_MemoryLoad/HW_Category_MemoryStore handling is added to genHWIntrinsic, the jump-table fallback, and StackLevelSetter (null-check throw helper). OperIsMemoryLoad/OperIsMemoryStore are enabled for TARGET_WASM and default to Op(1) for the address. Lowering marks the address multiply-used so a null check can be emitted, and the codegenwasm.cpp SIMD16 store-indirect NYI bailout is removed. A functional test verifies NullReferenceException is thrown for each new load/store on a null address.

Summary: LGTM. The change is internally consistent and cohesive: the instruction-list entries, categories, flags, base-type derivation, and codegen paths all line up (e.g., HW_Flag_HasImmediateOperand is set exactly on the entries that carry a lane immediate, and needsJumpTableFallback now correctly returns false for immediate-less intrinsics). The HasImmediateOperand-flag refactor cleanly generalizes ContainCheckHWIntrinsic and the jump-table resultType correctly handles the TYP_VOID store case. Null-check emission (genEmitNullCheck on the address register, with SetThrowHelperBlock(SCK_NULL_CHECK) registered in StackLevelSetter) is wired end-to-end and exercised by the new LoadStoreNullCheckTest. Coverage for the runtime jump-table path (non-constant lane index) and the newly-enabled SIMD16 store-indirect path relies on existing behavioral tests plus WASM CI; nothing in the diff appears incorrect. The PR already carries approvals from JIT-area experts and this review concurs.

Note

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

Generated by Holistic Review · 138.1 AIC · ⌖ 10.7 AIC · ⊞ 10K

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/jit/stacklevelsetter.cpp:280

• The #endif comment and directive structure are now a bit hard to follow: the opening #if is XARCH-only, but the closing #endif comment references both XARCH and WASM. The repo convention is to keep #else/#endif comments aligned with the exact opening directive; consider collapsing into a single #if defined(FEATURE_HW_INTRINSICS) && (defined(TARGET_XARCH) || defined(TARGET_WASM)) with nested #if defined(TARGET_*) inside one case GT_HWINTRINSIC:.

#elif defined(FEATURE_HW_INTRINSICS) && defined(TARGET_WASM)
        case GT_HWINTRINSIC:
        {
            HWIntrinsicCategory category = HWIntrinsicInfo::lookupCategory(node->AsHWIntrinsic()->GetHWIntrinsicId());
            if (category == HW_Category_MemoryLoad || category == HW_Category_MemoryStore)

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/tests/JIT/HardwareIntrinsics/Wasm/PackedSimd/PackedSimdTests.cs:1084

• The new test uses constant lane index 2 for LoadScalarAndInsert/StoreSelectedScalar, which doesn’t exercise the non-constant immediate path that triggers the jump-table fallback added for Wasm lane load/store intrinsics. Consider using a runtime-computed index (kept in-range) so this test also validates the jump-table codegen doesn’t produce invalid Wasm for non-const lane operands.

        Assert.Throws<NullReferenceException>(() => LoadScalarAndSplatVector128(null));
        Assert.Throws<NullReferenceException>(() => LoadScalarVector128(null));
        Assert.Throws<NullReferenceException>(() => LoadWideningVector128(null));
        Assert.Throws<NullReferenceException>(() => LoadScalarAndInsert(null, 2));
        Assert.Throws<NullReferenceException>(() => StoreSelectedScalar(null, 2));

[github-actions]

Holistic Review

Motivation: Extend the RyuJIT WebAssembly backend to support SIMD element-wise loads and stores, wiring up LoadScalarAndInsert, LoadScalarVector128, LoadScalarAndSplatVector128, LoadWideningVector128, and StoreSelectedScalar as real code-generating intrinsics (including jump-table fallback for LoadScalarAndInsert/StoreSelectedScalar when the lane index is not a compile-time constant). Previously these were HW_Flag_InvalidNodeId|HW_Flag_SpecialImport placeholders or NYI paths.

Approach: The change decouples "has an immediate operand" from the HW_Category_IMM category by adding a new HW_Flag_HasImmediateOperand flag for WASM (mirroring the existing ARM64 model), so HasImmediateOperand, needsJumpTableFallback, and ContainCheckHWIntrinsic key off the flag rather than the category. HW_Category_MemoryLoad/HW_Category_MemoryStore handling is added to genHWIntrinsic, the jump-table fallback, and StackLevelSetter (null-check throw helper). OperIsMemoryLoad/OperIsMemoryStore are enabled for TARGET_WASM and default to Op(1) for the address. Lowering marks the address multiply-used so a null check can be emitted, and the codegenwasm.cpp SIMD16 store-indirect NYI bailout is removed. A functional test verifies NullReferenceException is thrown for each new load/store on a null address.

Summary: LGTM (unchanged). The only change since the prior review is a jit-format commit that reflows the local declaration bool isMem = ... into bool isMem = ... in hwintrinsiccodegenwasm.cpp for column alignment. This is a pure whitespace/formatting change with no semantic effect, and the range-diff confirms commits 1–8 are byte-identical. The cumulative assessment is therefore unchanged: instruction-list entries, categories, flags, base-type derivation, and codegen paths remain consistent, and null-check emission is wired end-to-end and exercised by LoadStoreNullCheckTest. No new actionable findings.

Assessment History:

• review 4730813646 reviewed commit e78535b with verdict LGTM. Current verdict is LGTM. Assessment is unchanged — the only added patch is a formatting-only jit-format commit with no impact on motivation, approach, or risk.

Note

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

Generated by Holistic Review · 38.9 AIC · ⌖ 16.2 AIC · ⊞ 10K

[github-actions]

Holistic Review

Motivation: Extend the RyuJIT WebAssembly backend to support SIMD element-wise loads and stores, wiring up LoadScalarAndInsert, LoadScalarVector128, LoadScalarAndSplatVector128, LoadWideningVector128, and StoreSelectedScalar as real code-generating intrinsics (with jump-table fallback for the immediate-carrying ones when the lane index is not a compile-time constant), replacing prior HW_Flag_InvalidNodeId|HW_Flag_SpecialImport placeholders and NYI paths.

Approach: A new HW_Flag_HasImmediateOperand flag decouples "has an immediate operand" from HW_Category_IMM (mirroring ARM64), so HasImmediateOperand, needsJumpTableFallback, and ContainCheckHWIntrinsic key off the flag. HW_Category_MemoryLoad/MemoryStore handling is added to genHWIntrinsic, the jump-table fallback, and StackLevelSetter; OperIsMemoryLoad/OperIsMemoryStore are enabled for TARGET_WASM; lowering marks the address multiply-used to emit a null check; and the SIMD16 store-indirect NYI bailout is removed. A functional test verifies NullReferenceException on null addresses.

Summary: LGTM. Since the prior review, the only new patch is a jit-format whitespace/alignment change to a single bool isMem declaration in hwintrinsiccodegenwasm.cpp (variable-name and initializer alignment). It carries no semantic effect and does not alter behavior, control flow, or correctness. No new actionable findings. The cumulative assessment is unchanged: the intrinsic-list entries, categories, flags, base-type derivation, codegen, and null-check wiring remain internally consistent and cohesive.

Assessment History:

• review 4730813646 (commit e78535b): verdict LGTM. Current verdict LGTM. Unchanged — the incremental jit-format commit is formatting-only and does not affect motivation, approach, or risk.

Note

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

Generated by Holistic Review · 46.8 AIC · ⌖ 9.78 AIC · ⊞ 10K

[adamperlin]

/ba-g infra failure (artifact upload)