[wasm][coreclr] Fix reflection with large valuetypes #123759
Conversation
|
I am also writing down my thoughts on #120791 and will add them soon |
There was a problem hiding this comment.
Pull request overview
This PR updates the WASM/CoreCLR interpreter calling convention support so reflection invoke can correctly pass/return large value types (notably SIMD vectors), including adjusting the special-argument ordering (retbuf vs this) and improving stack layout alignment.
Changes:
- Adjust WASM reflection invoke/call-descriptor plumbing to account for retbuf presence and interpreter stack-slot sizing.
- Update ArgIterator WASM logic for special-arg offsets and value-type alignment handling.
- Fix a test failure message to correctly reference
Vector128<T>.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/JIT/Directed/VectorABI/VectorMgdMgdArray.cs | Updates failure message text to reflect Vector128<T> checks. |
| src/coreclr/vm/wasm/cgencpu.h | Refines return-type size macros used by calling convention logic. |
| src/coreclr/vm/wasm/calldescrworkerwasm.cpp | Adjusts interpreter call argument pointer/size handling when a retbuf is present. |
| src/coreclr/vm/reflectioninvocation.cpp | Updates stack-slot counting and records retbuf presence for WASM reflection calls. |
| src/coreclr/vm/frames.cpp | Recomputes this offset via ArgIterator instance (signature-aware). |
| src/coreclr/vm/callingconvention.h | Makes GetThisOffset instance-based; updates WASM special-arg ordering and adds valuetype alignment logic. |
| src/coreclr/vm/callhelpers.h | Adds hasRetBuff field (WASM-only) to CallDescrData. |
| src/coreclr/vm/callhelpers.cpp | Initializes hasRetBuff for WASM call paths and adds a debug assert. |
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
|
GCC leg has build break |
Indeed. Should be fine now. |
| @@ -26,5 +28,24 @@ extern "C" void STDCALL CallDescrWorkerInternal(CallDescrData* pCallDescrData) | |||
| targetIp = pMethod->GetInterpreterCode(); | |||
There was a problem hiding this comment.
(unrelated to this PR) Just a reminted that WASM-TODO a few lines above is a farm for intermittent crashes. Ideally, it would be fixed by us getting rid of CallDescrWorker completely. If we are not able to do that soon enough, we may need to figure an alternative solution. cc @AaronRobinsonMSFT
There was a problem hiding this comment.
Copilot and I are working on it :)
There was a problem hiding this comment.
@AaronRobinsonMSFT how do you plan to solve it?
Implement return of large valuetypes from reflection calls and also large valuetypes arguments. Change the order of `this` and `retbuf` arguments. This goes toward the dotnet#120791 and the new calling convention. It also helps to keep the interpreter calls use simple memory copy. JIT/Directed/Directed_3 runtime tests were used as repro. It fixes 6 tests in Pri0 and ~2000 tests in Pri1 Fixes dotnet#122922 --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Jan Kotas <jkotas@microsoft.com>
6 participants
Implement return of large valuetypes from reflection calls and also large valuetypes arguments.
Change the order of
thisandretbufarguments. This goes toward the #120791 and the new calling convention. It also helps to keep the interpreter calls use simple memory copy.JIT/Directed/Directed_3 runtime tests were used as repro.
It fixes 6 tests in Pri0 and ~2000 tests in Pri1
Fixes #122922