Skip to content
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

<type_traits>: __vectorcall _should_ be rejected on ARM64EC but isn't #4596

Closed
CaseyCarter opened this issue Apr 16, 2024 · 1 comment · Fixed by #4600
Closed

<type_traits>: __vectorcall _should_ be rejected on ARM64EC but isn't #4596

CaseyCarter opened this issue Apr 16, 2024 · 1 comment · Fixed by #4600
Labels
ARM64 Related to the ARM64 architecture bug Something isn't working fixed Something works now, yay!

Comments

@CaseyCarter
Copy link
Member

Per https://learn.microsoft.com/en-us/cpp/cpp/vectorcall?view=msvc-170:

On ARM64EC, __vectorcall is unsupported and rejected by the compiler.

but we use __vectorcall on ARM64EC:

STL/stl/inc/type_traits

Lines 398 to 402 in 9aca224

#if ((defined(_M_IX86) && _M_IX86_FP >= 2) || defined(_M_X64)) && !defined(_M_CEE)
#define _EMIT_VECTORCALL(FUNC, OPT1, OPT2, OPT3) FUNC(__vectorcall, OPT1, OPT2, OPT3)
#else // ^^^ __vectorcall supported / __vectorcall not supported vvv
#define _EMIT_VECTORCALL(FUNC, OPT1, OPT2, OPT3)
#endif // ^^^ __vectorcall not supported ^^^

I'm not sure what effect this has. For all I know the compiler specifically doesn't reject our usage of __vectorcall just to support the STL. Removal is simple enough - see the patch below - but before simply yanking it I think we should investigate with the compiler team and ensure nothing actually useful is happening here. Thanks to llvm/llvm-project#87725 (comment) for bringing this to my attention.

diff --git a/stl/inc/type_traits b/stl/inc/type_traits
index 43e2ca6a..3adf3078 100644
--- a/stl/inc/type_traits
+++ b/stl/inc/type_traits
@@ -395,7 +395,7 @@ constexpr bool is_compound_v = !is_fundamental_v<_Ty>;
 #define _EMIT_THISCALL(FUNC, OPT1, OPT2, OPT3)
 #endif // ^^^ __stdcall and __thiscall not supported ^^^
 
-#if ((defined(_M_IX86) && _M_IX86_FP >= 2) || defined(_M_X64)) && !defined(_M_CEE)
+#if ((defined(_M_IX86) && _M_IX86_FP >= 2) || (defined(_M_X64) && !defined(_M_ARM64EC))) && !defined(_M_CEE)
 #define _EMIT_VECTORCALL(FUNC, OPT1, OPT2, OPT3) FUNC(__vectorcall, OPT1, OPT2, OPT3)
 #else // ^^^ __vectorcall supported / __vectorcall not supported vvv
 #define _EMIT_VECTORCALL(FUNC, OPT1, OPT2, OPT3)
diff --git a/tests/std/tests/Dev09_158457_tr1_mem_fn_calling_conventions/test.cpp b/tests/std/tests/Dev09_158457_tr1_mem_fn_calling_conventions/test.cpp
index 6f3576cc..0187f23f 100644
--- a/tests/std/tests/Dev09_158457_tr1_mem_fn_calling_conventions/test.cpp
+++ b/tests/std/tests/Dev09_158457_tr1_mem_fn_calling_conventions/test.cpp
@@ -64,7 +64,7 @@ struct Cat {
     }
 #endif
 
-#if (defined(_M_IX86) || defined(_M_X64)) && !defined(_M_CEE)
+#if (defined(_M_IX86) || (defined(_M_X64) && !defined(_M_ARM64EC))) && !defined(_M_CEE)
     int __vectorcall g() {
         return 2;
     }
@@ -115,7 +115,7 @@ int y(int i) {
 }
 #endif
 
-#if (defined(_M_IX86) || defined(_M_X64)) && !defined(_M_CEE)
+#if (defined(_M_IX86) || (defined(_M_X64) && !defined(_M_ARM64EC))) && !defined(_M_CEE)
 int __vectorcall z(int i) {
     return -4 * i;
 }
diff --git a/tests/std/tests/P0898R3_concepts/test.cpp b/tests/std/tests/P0898R3_concepts/test.cpp
index 25d325f8..6cbfd189 100644
--- a/tests/std/tests/P0898R3_concepts/test.cpp
+++ b/tests/std/tests/P0898R3_concepts/test.cpp
@@ -2879,12 +2879,12 @@ namespace test_invocable_concepts {
 #include "invocable_cc.hpp"
 
 #ifndef _M_CEE // avoid warning C4575: '__vectorcall' incompatible with the '/clr' option: converting to '__stdcall'
-#if !defined(_M_ARM) && !defined(_M_ARM64)
+#if !defined(_M_ARM) && !defined(_M_ARM64) && !defined(_M_ARM64EC)
 #define NAME      test_vector_vector
 #define CALLCONV  __vectorcall
 #define MCALLCONV __vectorcall
 #include "invocable_cc.hpp"
-#endif // ^^^ !ARM && !ARM64 ^^^
+#endif // ^^^ !ARM && !ARM64 && !ARM64EC ^^^
 #endif // _M_CEE
 
 } // namespace test_invocable_concepts
@CaseyCarter CaseyCarter added the question Further information is requested label Apr 16, 2024
@StephanTLavavej StephanTLavavej added info needed We need more info before working on this ARM64 Related to the ARM64 architecture compiler Compiler work involved bug Something isn't working and removed question Further information is requested info needed We need more info before working on this compiler Compiler work involved labels Apr 17, 2024
@StephanTLavavej
Copy link
Member

We got confirmation from the compiler devs: __vectorcall is currently not supported for ARM64EC, and our usage should be guarded. Support for it is expected at some point in the future but it's low priority and there's no ETA.

CaseyCarter added a commit to CaseyCarter/STL that referenced this issue Apr 18, 2024
The documentation says "On ARM64EC, `__vectorcall` is unsupported and rejected by the compiler." I'm not sure why our usage isn't being rejected, but we have confirmation from the compiler team that ARM64 doesn't currently accept `__vectorcall` so we should avoid it.

Fixes microsoft#4596
@StephanTLavavej StephanTLavavej added the fixed Something works now, yay! label Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ARM64 Related to the ARM64 architecture bug Something isn't working fixed Something works now, yay!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants