[PATCH 0/3] MR3986: include: Initial ARM64EC target support.
ARM64EC target defines x86_64 macros to ensure that x86_64-specific code paths are used, but for code that really needs to use aarch64 branch there is an additional _M_ARM64EC macro that needs to be used. See https://techcommunity.microsoft.com/t5/windows-os-platform-blog/getting-to-k... for details. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3986
From: Jacek Caban <jacek(a)codeweavers.com> --- include/msvcrt/intrin.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/msvcrt/intrin.h b/include/msvcrt/intrin.h index 8b84929bc02..c66ffff8687 100644 --- a/include/msvcrt/intrin.h +++ b/include/msvcrt/intrin.h @@ -7,7 +7,7 @@ #ifndef _INC_INTRIN #define _INC_INTRIN -#if defined(__i386__) || defined(__x86_64__) +#if defined(__i386__) || (defined(__x86_64__) && !defined(_M_ARM64EC)) # include <x86intrin.h> #endif @@ -15,7 +15,7 @@ extern "C" { #endif -#if defined(__i386__) || defined(__x86_64__) +#if defined(__i386__) || (defined(__x86_64__) && !defined(_M_ARM64EC)) static inline void __cpuidex(int info[4], int ax, int cx) { __asm__ ("cpuid" : "=a"(info[0]), "=b" (info[1]), "=c"(info[2]), "=d"(info[3]) : "a"(ax), "c"(cx)); @@ -26,7 +26,7 @@ static inline void __cpuid(int info[4], int ax) } #endif -#ifdef __aarch64__ +#if defined(__aarch64__) || defined(_M_ARM64EC) typedef enum _tag_ARM64INTR_BARRIER_TYPE { _ARM64_BARRIER_OSHLD = 0x1, @@ -58,7 +58,7 @@ typedef enum _tag_ARMINTR_BARRIER_TYPE } _ARMINTR_BARRIER_TYPE; #endif -#if defined(_MSC_VER) && (defined(__arm__) || defined(__aarch64__)) +#if defined(_MSC_VER) && (defined(__arm__) || defined(__aarch64__) || defined(_M_ARM64EC)) void __dmb(unsigned int); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3986
From: Jacek Caban <jacek(a)codeweavers.com> --- include/msvcrt/intrin.h | 7 +++++++ include/winnt.h | 2 -- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/include/msvcrt/intrin.h b/include/msvcrt/intrin.h index c66ffff8687..88af8ae71ea 100644 --- a/include/msvcrt/intrin.h +++ b/include/msvcrt/intrin.h @@ -66,6 +66,13 @@ void __dmb(unsigned int); #endif +#if defined(_MSC_VER) && (defined(__aarch64__) || defined(_M_ARM64EC)) + +unsigned __int64 __getReg(int); +#pragma intrinsic(__getReg) + +#endif + #ifdef __cplusplus } #endif diff --git a/include/winnt.h b/include/winnt.h index df0c3282f8c..79fa8d6077b 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -2439,8 +2439,6 @@ static FORCEINLINE struct _TEB * WINAPI NtCurrentTeb(void) return __wine_current_teb; } #elif defined(__aarch64__) && defined(_MSC_VER) -unsigned __int64 __getReg(int); -#pragma intrinsic(__getReg) static FORCEINLINE struct _TEB * WINAPI NtCurrentTeb(void) { return (struct _TEB *)__getReg(18); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3986
From: Jacek Caban <jacek(a)codeweavers.com> --- include/winnt.h | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/include/winnt.h b/include/winnt.h index 79fa8d6077b..f2050e1a89b 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -33,7 +33,7 @@ #endif -#if defined(_MSC_VER) && (defined(__arm__) || defined(__aarch64__)) +#if defined(_MSC_VER) && (defined(__arm__) || defined(__aarch64__) || defined(_M_ARM64EC)) #include <intrin.h> #endif @@ -2405,6 +2405,17 @@ static FORCEINLINE struct _TEB * WINAPI NtCurrentTeb(void) __asm mov teb, eax; return teb; } +#elif (defined(__aarch64__) || defined(_M_ARM64EC)) && defined(__GNUC__) +register struct _TEB *__wine_current_teb __asm__("x18"); +static FORCEINLINE struct _TEB * WINAPI NtCurrentTeb(void) +{ + return __wine_current_teb; +} +#elif (defined(__aarch64__) || defined(_M_ARM64EC)) && defined(_MSC_VER) +static FORCEINLINE struct _TEB * WINAPI NtCurrentTeb(void) +{ + return (struct _TEB *)__getReg(18); +} #elif defined(__x86_64__) && defined(__GNUC__) static FORCEINLINE struct _TEB * WINAPI NtCurrentTeb(void) { @@ -2432,17 +2443,6 @@ static FORCEINLINE struct _TEB * WINAPI NtCurrentTeb(void) { return (struct _TEB *)(ULONG_PTR)_MoveFromCoprocessor(15, 0, 13, 0, 2); } -#elif defined(__aarch64__) && defined(__GNUC__) -register struct _TEB *__wine_current_teb __asm__("x18"); -static FORCEINLINE struct _TEB * WINAPI NtCurrentTeb(void) -{ - return __wine_current_teb; -} -#elif defined(__aarch64__) && defined(_MSC_VER) -static FORCEINLINE struct _TEB * WINAPI NtCurrentTeb(void) -{ - return (struct _TEB *)__getReg(18); -} #elif !defined(RC_INVOKED) # error You must define NtCurrentTeb() for your architecture #endif @@ -6902,6 +6902,13 @@ static FORCEINLINE void MemoryBarrier(void) InterlockedOr(&dummy, 0); } +#elif defined(__aarch64__) || defined(_M_ARM64EC) + +static FORCEINLINE void MemoryBarrier(void) +{ + __dmb(_ARM64_BARRIER_SY); +} + #elif defined(__x86_64__) #pragma intrinsic(__faststorefence) @@ -6919,13 +6926,6 @@ static FORCEINLINE void MemoryBarrier(void) __dmb(_ARM_BARRIER_SY); } -#elif defined(__aarch64__) - -static FORCEINLINE void MemoryBarrier(void) -{ - __dmb(_ARM64_BARRIER_SY); -} - #endif /* __i386__ */ /* Since Visual Studio 2012, volatile accesses do not always imply acquire and @@ -7191,7 +7191,7 @@ unsigned char _InterlockedCompareExchange128(volatile __int64 *, __int64, __int6 static FORCEINLINE unsigned char InterlockedCompareExchange128( volatile __int64 *dest, __int64 xchg_high, __int64 xchg_low, __int64 *compare ) { -#ifdef __x86_64__ +#if defined(__x86_64__) && !defined(_M_ARM64EC) unsigned char ret; __asm__ __volatile__( "lock cmpxchg16b %0; setz %b2" : "=m" (dest[0]), "=m" (dest[1]), "=r" (ret), -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3986
We tend to use the Unix names for such things, shouldn't there be an `__arm64ec__` define instead? Or maybe we should switch to using the _M_ defines for all platforms... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3986#note_47191
On Fri Sep 29 11:28:24 2023 +0000, Alexandre Julliard wrote:
We tend to use the Unix names for such things, shouldn't there be an `__arm64ec__` define instead? Or maybe we should switch to using the _M_ defines for all platforms... I was wondering about how those macros should work. AFAICT there is no `__arm64ec__` macro on MSVC, it was invented recently in clang by @bylaws: https://github.com/llvm/llvm-project/pull/65420. From compiler's perspective defining macros like __aarch64__ makes sense, because it's a standard everywhere outside MSVC. However, there is nothing to be compatible with in case of ARM64EC. In practice, this means that there will be an extra unneeded difference between mingw and MSVC. I'm tempted to suggest removing __arm64ec__ from clang before backward compatibility is an issue, but I'm not sure about that so opinions are welcomed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3986#note_47192
On Fri Sep 29 11:33:53 2023 +0000, Jacek Caban wrote:
I was wondering about how those macros should work. AFAICT there is no `__arm64ec__` macro on MSVC, it was invented recently in clang by @bylaws: https://github.com/llvm/llvm-project/pull/65420. From compiler's perspective defining macros like `__aarch64__` makes sense, because it's a standard everywhere outside MSVC. However, there is nothing to be compatible with in case of ARM64EC. In practice, this means that there will be an extra unneeded difference between mingw and MSVC. I'm tempted to suggest removing `__arm64ec__` from clang before backward compatibility is an issue, but I'm not sure about that so opinions are welcomed. I also generally dislike unnecessary differences between mingw and MSVC environments, but this one is quite clear IMO; in mingw mode, the compiler shouldn't predefine any `_M_<arch>` defines, but it should do that in the `__<arch>__` namespace. But here there's indeed no predecent for what to name it, so we do need one. And `__aarch64ec__` isn't better, since there's literally no such thing as `aarch64ec`.
@julliard So yes, there is an `__arm64ec__` define that we could check for, in mingw contexts. (For MSVC contexts, I guess we should define that based on `_M_ARM64EC`, if that's what's done for other architecture macros? Or does Wine carry parallel checks for GNU and MSVC style arch macros everywhere throughout?) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3986#note_47193
On Fri Sep 29 11:39:56 2023 +0000, Martin Storsjö wrote:
I also generally dislike unnecessary differences between mingw and MSVC environments, but this one is quite clear IMO; in mingw mode, the compiler shouldn't predefine any `_M_<arch>` defines, but it should do that in the `__<arch>__` namespace. But here there's indeed no predecent for what to name it, so we do need one. And `__aarch64ec__` isn't better, since there's literally no such thing as `aarch64ec`. @julliard So yes, there is an `__arm64ec__` define that we could check for, in mingw contexts. (For MSVC contexts, I guess we should define that based on `_M_ARM64EC`, if that's what's done for other architecture macros? Or does Wine carry parallel checks for GNU and MSVC style arch macros everywhere throughout?) Wine uses configure to ensure that `__<arch>__` macros are present: https://gitlab.winehq.org/wine/wine/-/blob/master/configure.ac?ref_type=head...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3986#note_47195
On Fri Sep 29 11:49:27 2023 +0000, Jacek Caban wrote:
Wine uses configure to ensure that `__<arch>__` macros are present: https://gitlab.winehq.org/wine/wine/-/blob/master/configure.ac?ref_type=head... Thanks - I just found the same when checking around.
However for the PE/ELF split, we only execute that check for the ELF side host compiler, it seems. When compiling with clang in MSVC mode, we never execute that check. (Perhaps we should?) However, even in MSVC mode, Clang does define `__aarch64__`, in addition to `_M_ARM64`. So perhaps we should do the same for `__arm64ec__` as well? Or perhaps that's already done by the commit by @bylaws - I think that's done based on my reading of that commit, but I haven't tested it out myself. So I guess that just leaves the case of compiling with actual MSVC (which I believe is kinda relevant now, given the current incomplete arm64ec support in LLVM/Clang). I guess we could/should extend the `WINE_CHECK_DEFINE` handling for the PE compiler as well, and consistently use the GNU style naming? But I guess that requires knowing more about arm64ec in configure, which we don't do right now (but I presume we'll need sooner or later anyway)? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3986#note_47196
On Fri Sep 29 11:55:06 2023 +0000, Martin Storsjö wrote:
Thanks - I just found the same when checking around. However for the PE/ELF split, we only execute that check for the ELF side host compiler, it seems. When compiling with clang in MSVC mode, we never execute that check. (Perhaps we should?) However, even in MSVC mode, Clang does define `__aarch64__`, in addition to `_M_ARM64`. So perhaps we should do the same for `__arm64ec__` as well? Or perhaps that's already done by the commit by @bylaws - I think that's done based on my reading of that commit, but I haven't tested it out myself. So I guess that just leaves the case of compiling with actual MSVC (which I believe is kinda relevant now, given the current incomplete arm64ec support in LLVM/Clang). I guess we could/should extend the `WINE_CHECK_DEFINE` handling for the PE compiler as well, and consistently use the GNU style naming? But I guess that requires knowing more about arm64ec in configure, which we don't do right now (but I presume we'll need sooner or later anyway)? I wasn't plan using the actual MSVC with Wine build system myself, but yes, in theory we could support that and something like you suggest would be needed then.
And yes, `__arm64ec__` is already defined in addition to `_M_ARM64EC` by clang for MSVC targets. BTW, I always considered handling of `_M_<arch>` macros in mingw to be a hack working around GCC shortcomings. They are defined in a header, so they work only if you include any crt headers first, which is not what one could expect. I recall hitting it while porting code to mingw. While it's too late for other platforms, I just feel like we could do better for new ones. I find explicitly skipping `_M_<arch>` macro in clang for mingw targets only to define it in a less reliable way in mingw headers to be sub-optimal. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3986#note_47197
On Fri Sep 29 12:27:37 2023 +0000, Jacek Caban wrote:
I wasn't plan using the actual MSVC with Wine build system myself, but yes, in theory we could support that and something like you suggest would be needed then. And yes, `__arm64ec__` is already defined in addition to `_M_ARM64EC` by clang for MSVC targets. BTW, I always considered handling of `_M_<arch>` macros in mingw to be a hack working around GCC shortcomings. They are defined in a header, so they work only if you include any crt headers first, which is not what one could expect. I recall hitting it while porting code to mingw. While it's too late for other platforms, I just feel like we could do better for new ones. I find explicitly skipping `_M_<arch>` macro in clang for mingw targets only to define it in a less reliable way in mingw headers to be sub-optimal. Yeah, the fact that mingw headers do define `_M_<arch>` but in a header feels a bit indecisive to me; either mingw shouldn't be defining them, forcing everyone to be consistent in using the GNU style names - or they perhaps indeed should be a compiler built-in define.
I somewhat disagree that clang for mingw targets should have gone with defining these though - I think that'd lead to even more confusion. If we'd want to go that way, I think we perhaps should coordinate that with GCC and make both compilers switch to doing that. In any case, I think this is a separate topic from ARM64EC :-) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3986#note_47198
On Fri Sep 29 13:13:40 2023 +0000, Martin Storsjö wrote:
Yeah, the fact that mingw headers do define `_M_<arch>` but in a header feels a bit indecisive to me; either mingw shouldn't be defining them, forcing everyone to be consistent in using the GNU style names - or they perhaps indeed should be a compiler built-in define. I somewhat disagree that clang for mingw targets should have gone with defining these though - I think that'd lead to even more confusion. If we'd want to go that way, I think we perhaps should coordinate that with GCC and make both compilers switch to doing that. In any case, I think this is a separate topic from ARM64EC :-) I wasn't suggesting changing existing targets, I was only suggesting to not extend the problem to the new target while we still have freedom to do so.
Anyway, I will change those patches to use `__arm64ec__`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3986#note_47205
participants (4)
-
Alexandre Julliard (＠julliard) -
Jacek Caban
-
Jacek Caban (＠jacek)
-
Martin Storsjö