Fix HFA/HVA by-value argument marshalling on ARM64 in CallTargetWorker#130580
Fix HFA/HVA by-value argument marshalling on ARM64 in CallTargetWorker#130580steveisok wants to merge 1 commit into
Conversation
MethodDescCallSite::CallTargetWorker manually marshals arguments into the
register save area. The branch that expands a struct-passed-in-registers
into individual register slots was gated to UNIX_AMD64_ABI / LOONGARCH64 /
RISCV64 only. On ARM64, HFA/HVA structs passed by value in FP registers fell
through to the default packed CopyMemory, which lays the fields out
contiguously instead of placing each field in its own NEON register slot.
The callee then read {field0, field2, garbage, ...}.
Add a TARGET_ARM64 branch that calls the existing
ArgDestination::CopyHFAStructToRegister when the argument is an HFA, matching
the pattern already used in CopyValueClassArgUnchecked. Non-HFA structs (and
HFAs spilled to the stack) keep IsHFA() == false and fall through to the
unchanged default copy.
This manifested as "vector math broken in the debugger only" because normal
JIT-emitted calls and reflection invoke stubs do not use this manual
marshalling path, but debugger func-eval does.
Fixes dotnet#126564
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @agocke |
There was a problem hiding this comment.
Pull request overview
This PR fixes ARM64 by-value HFA/HVA argument marshalling in MethodDescCallSite::CallTargetWorker by expanding HFA/HVA struct data into the floating-point/vector register save slots, instead of copying the packed struct bytes contiguously. This aligns the manual marshalling path (notably used by debugger func-eval) with the ARM64 ABI’s “one field per FP/SIMD register” expectation.
Changes:
- Add a
TARGET_ARM64branch inCallTargetWorkerto detect HFA/HVA arguments and callArgDestination::CopyHFAStructToRegister. - Preserve the existing default copy behavior for non-HFA structs and for HFAs that are passed on the stack (i.e., not enregistered).
| if (argDest.IsHFA()) | ||
| { | ||
| // An HFA/HVA struct argument is enregistered with each field in its own | ||
| // floating-point/vector register. Expand the packed struct data into the | ||
| // register slots instead of copying it verbatim. | ||
| argDest.CopyHFAStructToRegister(pSrc, stackSize); | ||
| } |
Reflection invoke does use this path. If there is a bug, it should repro with reflection invoke too. We should add reflection invoke test that fails before this change and passes after this change. |
|
Ok, reflection invoke can hit this method, but it won't be able to hit this specific path. It explains why it is a debugger only bug |
MethodDescCallSite::CallTargetWorker manually marshals arguments into the register save area. The branch that expands a struct-passed-in-registers into individual register slots was gated to UNIX_AMD64_ABI / LOONGARCH64 / RISCV64 only. On ARM64, HFA/HVA structs passed by value in FP registers fell through to the default packed CopyMemory, which lays the fields out contiguously instead of placing each field in its own NEON register slot. The callee then read {field0, field2, garbage, ...}.
Add a TARGET_ARM64 branch that calls the existing
ArgDestination::CopyHFAStructToRegister when the argument is an HFA, matching the pattern already used in CopyValueClassArgUnchecked. Non-HFA structs (and HFAs spilled to the stack) keep IsHFA() == false and fall through to the unchanged default copy.
This manifested as "vector math broken in the debugger only" because normal JIT-emitted calls and reflection invoke stubs do not use this manual marshalling path, but debugger func-eval does.
Fixes #126564