Repository navigation
[wasm][JIT] Use TYP_SIMD16 register type for 16-byte struct layouts on Wasm - #134984
Merged
Merged
Conversation
…n Wasm Revert the Wasm exclusion added in #129494 now that Wasm SIMD16 local loads are no longer NYI. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 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 'arch-wasm': @lewing, @pavelsavara |
Member
Author
|
@adamperlin @AndyAyersMS this looks like a nice win? |
jakobbotsch
approved these changes
Oct 1, 2026
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 free
to 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.
Motivation
a17debe (#129494) excluded Wasm from the 16-byte
TYP_SIMD16case inClassLayout::GetRegisterType()while Wasm SIMD16 local loads were still NYI. Those SIMD NYIs are no longer present insrc/coreclr/jit/codegenwasm.cpp.Because of the exclusion, on Wasm a struct wrapping
Vector128<T>that is returned asv128(call typedsimd16) and stored to aTYP_STRUCTlocal haslclRegType == TYP_UNDEF, soLowering::LowerStoreLocCommonroutes it throughSpillStructCallResult: a new do-not-enregister temp, aSTORE_LCL_FLD simd16into it, then a struct copy into the destination. That path also crashed crossgen2 with a nullReturnTypeDescdereference (#134976, fixed separately in #134981). This PR removes the reason Wasm takes that path for v128 returns.Change
In
ClassLayout::GetRegisterType()(src/coreclr/jit/layout.h), change#if defined(FEATURE_SIMD) && !defined(TARGET_WASM)back to#ifdef FEATURE_SIMD, so 16-byte struct layouts getTYP_SIMD16as their register type on Wasm as on other SIMD targets.Validation
Performed in a sibling worktree: browser-wasm Checked, osx-arm64 host crossgen2, with the #134981 fix also applied.
./build.sh -s clr+libs -os browser -c checked -lc release: succeededsrc/tests/build.sh -browser checked priority1 -test:JIT/HardwareIntrinsics/HardwareIntrinsics_General_r.csproj -test:JIT/HardwareIntrinsics/HardwareIntrinsics_General_ro.csproj /p:LibrariesConfiguration=Release: succeededsrc/tests/run.sh wasm checked --runcrossgen2tests --node --runner-filter=HardwareIntrinsics_General(IL-CG2 dirs and .wasm images were deleted first to force recompilation):HardwareIntrinsics_General_r: 2584/2584 passedHardwareIntrinsics_General_ro: 2551/2551 passedVectorImmBinaryOpTest__op_LeftShiftByte1:RunStructLclFldScenario(Vector128_1_r): the JIT no longer creates theReturn value tempspill and stores thesimd16call result directly.Vector128_1_r.wasm: 4,957,655 → 4,951,047 bytes (-6.6 KB)Vector128_1_ro.wasm: 4,926,510 → 4,919,838 bytes (-6.7 KB)On this branch,
./build.sh clr.wasmjit -c checkedsucceeded.Caveat
Validation covered only the HardwareIntrinsics General runners. This change affects every 16-byte struct local on Wasm, so it needs broader Wasm R2R coverage (e.g. the outerloop R2R_CG2 browser-wasm lane) and sign-off from the Wasm JIT owners.
Related to #134976 and #134981
Note
This PR description was drafted with GitHub Copilot assistance.