-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix win-arm64 native varargs ABI #90712
Fix win-arm64 native varargs ABI #90712
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsSIMD vector types should be passed in integer registers. This might require splitting a Vector128 between x7 and stack.
|
Only failure is known failure #90593 |
No spmi asm diffs -- negligible TP change on arm64. |
@jakobbotsch PTAL |
Does VM-side implementation of calling conventions (https://github.com/dotnet/runtime/blob/main/src/coreclr/vm/callingconvention.h) work as expected for this case? |
if (varTypeIsSIMD(type)) | ||
{ | ||
// Vectors also get passed in int registers. Use TYP_INT. | ||
return TYP_INT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the size mismatch here fine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns out the size doesn't matter; the only thing that matters is INT (use general-purpose registers) or non-INT (use floaing-point/SIMD registers).
It looks like it might already be covered: runtime/src/coreclr/vm/callingconvention.h Lines 1618 to 1636 in 64243bb
One thing that wasn't handled on the JIT is passing/receiving any Vector to/in win-arm64 varargs functions. How could I test this case through the callingconvention.h code? |
callingconvention.h is only used for GC stackroot reporting. It may be useful to add a variant of the test with interleaving scalars and object refences in the vararg signature, so that we would catch the situation where the GC stackroot reporting does not report the varargs arguments correctly. |
SIMD vector types should be passed in integer registers. This might require splitting a Vector128 between x7 and stack.
fe32a89
to
1363246
Compare
I stepped through callingconvention.h and saw the correct behavior occuring with the current code. I added a few addtional test cases to the test. |
src/tests/JIT/Regression/JitBlue/Runtime_71375/Runtime_71375.cs
Outdated
Show resolved
Hide resolved
Add additional varargs arguments after the vector Co-authored-by: Jan Kotas <jkotas@microsoft.com>
SIMD vector types should be passed in integer registers for win-arm64 native varargs. This might require splitting a Vector128 between x7 and stack.
See https://learn.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=msvc-170#addendum-variadic-functions
Fixes #89595