[wasm] Emit the 'V' thunk signature-key char for Vector128<T>/Vector<T> args#131246
Conversation
…T> args
The interp<->R2R thunk ABI encodes a wasm v128 argument as the signature-key char
'V'. crossgen2 (WasmLowering) already generates interp->R2R thunks that encode a
supported Vector128<T> / 16-byte Vector<T> parameter as v128 => 'V'. But the VM-side
signature-key builder in helpers.cpp classified such a 16-byte vector as a generic
by-ref struct ('S16'), so at runtime it built a key that did not match the
crossgen2-generated 'V' thunk:
crossgen2 thunk key: ...V...
VM runtime lookup: ...S16... -> LookupThunk returns NULL
-> "WASM calli missing for key" (missing thunk)
for any interp->R2R call passing a Vector128<T> (or a 16-byte Vector<T>) by value.
Classify a supported vector base type (an intrinsic type with a single numeric generic
argument) that is Vector128<T>, or a 16-byte Vector<T>, as the new ConvertType::ToV128,
emitted as 'V' -- matching WasmLowering's v128 encoding -- so the runtime resolves the
correct v128-passing thunk. Mirrors the JIT's TYP_SIMD16 recognition and
WasmLowering.IsWasmV128Type.
Verified: WASI CoreCLR build (clr -os wasi -c Release) 0W/0E; validated against the
System.Runtime.CompilerServices.Unsafe library suite under R2R (128/128), which
exercises Vector128 interp<->R2R boundaries.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2902374a-f352-4c46-bcec-8a35c97733ae
|
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. |
|
Tagging subscribers to this area: @agocke |
|
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. |
There was a problem hiding this comment.
Pull request overview
This PR updates the CoreCLR wasm thunk signature-key builder so that by-value Vector128<T> (and 16-byte Vector<T>) parameters are classified as wasm v128 and encoded using the 'V' signature-key character, aligning VM runtime thunk lookup with crossgen2’s thunk key generation.
Changes:
- Add a new
ConvertType::ToV128classification for supported vector base types. - Detect
Vector128<T>and 16-byteVector<T>(with supported numericT) asv128inLowerTypeHandle. - Emit
'V'for the newToV128case when building the signature key.
|
cc @dotnet/wasm-eng |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/coreclr/vm/wasm/helpers.cpp:1042
CoreLibBinder::GetClassIfExist(CLASS__VECTOR128T)/GetClassIfExist(CLASS__VECTORT)are called unconditionally for every valuetype processed byLowerTypeHandle, even though the result is only needed when the type is an intrinsic vector candidate. SinceGetSignatureKeyruns underGC_NOTRIGGER, it’s best to minimize binder lookups here; you can move these lookups inside theisSupportedVectorBaseTypecheck, and only queryCLASS__VECTORTwhensize == 16.
PTR_MethodTable pVector128MT = CoreLibBinder::GetClassIfExist(CLASS__VECTOR128T);
PTR_MethodTable pVectorTMT = CoreLibBinder::GetClassIfExist(CLASS__VECTORT);
bool isSupportedVectorBaseType =
pMT->IsIntrinsicType() &&
(pMT->GetNumGenericArgs() == 1) &&
CorIsNumericalType(pMT->GetInstantiation()[0].GetSignatureCorElementType());
src/coreclr/vm/wasm/helpers.cpp:1126
GetSignatureKeyreferencesdocs/design/coreclr/botr/readytorun-format.mdas the documentation for wasm signature string encoding, but that doc currently doesn’t list theVencoding forv128in the return/parameter tables. Since this change introduces/usesV, the docs should be updated to include it so the three codebases stay aligned.
case ConvertType::ToF32: c = 'f'; break;
case ConvertType::ToF64: c = 'd'; break;
case ConvertType::ToV128: c = 'V'; break;
case ConvertType::ToEmpty: c = 'e'; break;
…at.md Add the 'V' token to the return-type and explicit-parameter tables of the "Wasm Signature String Encoding" section. 'V' encodes a wasm v128 value (Vector128<T>, or a 16-byte Vector<T>) passed/returned by value, matching crossgen2's WasmLowering (WasmValueType.V128 => 'V') and the CoreCLR runtime key-builder. Keeps the shared-encoding doc in sync with the three codebases. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2902374a-f352-4c46-bcec-8a35c97733ae
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
docs/design/coreclr/botr/readytorun-format.md:1089
- Same as above: consider using "e.g." so the
Vparameter description doesn't imply onlyVector128<T>/Vector<T>can produce av128lowering (wrapper structs can too).
| `V` | `v128` parameter (a `Vector128<T>`, or a 16-byte `Vector<T>`, passed by value) |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
I forgot merging a suggestion would require another reivew. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/coreclr/vm/wasm/helpers.cpp:1040
- The v128 classification probe runs for any intrinsic generic value type with a single numeric generic argument (e.g., Span, ByReference), even though only 16-byte types can ever lower to wasm v128. Adding a
size == 16guard avoids twoCoreLibBinder::GetClassIfExist(...)lookups on non-v128 types while keeping Vector128/Vector behavior unchanged.
bool isSupportedVectorBaseType =
pMT->IsIntrinsicType() &&
(pMT->GetNumGenericArgs() == 1) &&
CorIsNumericalType(pMT->GetInstantiation()[0].GetSignatureCorElementType());
docs/design/coreclr/botr/readytorun-format.md:1062
- This section says the encoding is "shared across three codebases", but the CoreCLR WasmAppBuilder
SignatureMapperimplementation currently doesn’t emit/parse the newly documentedV(v128) token. Either update WasmAppBuilder to handleV, or clarify here that WasmAppBuilder currently implements only a subset of the encoding.
| `V` | returns `v128` (a `Vector128<T>`, or a 16-byte `Vector<T>`) |
Summary
The interp↔R2R thunk ABI encodes a wasm
v128argument as the signature-key char'V'. crossgen2 (WasmLowering) already generates interp→R2R thunks that encode a supportedVector128<T>/ 16-byteVector<T>parameter asv128→'V'(WasmValueType.V128 => 'V', predicateIsWasmV128Type).But the VM-side signature-key builder in
src/coreclr/vm/wasm/helpers.cppclassified such a 16-byte vector as a generic by-ref struct (ToStruct→'S16'), so at runtime the VM built a key that did not match the crossgen2-generated'V'thunk:for any interp→R2R call passing a
Vector128<T>(or a 16-byteVector<T>) by value.Fix
Classify a supported vector base type — an intrinsic type with a single numeric generic argument that is
Vector128<T>, or a 16-byteVector<T>— as the newConvertType::ToV128, emitted as'V', matchingWasmLowering's v128 encoding so the runtime resolves the correct v128-passing thunk. Single-field structs wrapping a v128 recurse through the existing single-field unwrap path and land onToV128as well, matching crossgen2. Mirrors the JIT'sTYP_SIMD16recognition andWasmLowering.IsWasmV128Type.Validation
clr -os wasi -c Release): 0W/0E;helpers.cppobject recompiles clean.System.Runtime.CompilerServices.Unsafelibrary suite under R2R (128/128), which exercisesVector128interp↔R2R boundaries (previously failed with a missing thunk).Note
This PR was authored with the assistance of GitHub Copilot.