| Message ID | VI1PR83MB0431809D3F653B39481E740DF8572@VI1PR83MB0431.EURPRD83.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add aarch64-w64-mingw32 target | expand |
On 21/02/2024 18:30, Evgeny Karpov wrote:
>
+/* X18 reserved for the TEB on Windows. */
+#ifdef TARGET_ARM64_MS_ABI
+# define FIXED_X18 1
+# define CALL_USED_X18 0
+#else
+# define FIXED_X18 0
+# define CALL_USED_X18 1
+#endif
I'm not overly keen on ifdefs like this (and the one below), it can get quite confusing if we have to support more than a couple of ABIs. Perhaps we could create a couple of new headers, one for the EABI (which all existing targets would then need to include) and one for the MS ABI. Then the mingw port would use that instead of the EABI header.
An alternative is to make all this dynamic, based on the setting of the aarch64_calling_abi enum and to make the adjustments in aarch64_conditional_register_usage.
+# define CALL_USED_X18 0
Is that really correct? If the register is really reserved, but some code modifies it anyway, this will cause the compiler to restore the old value at the end of a function; generally, for a reserved register, code that knows what it's doing would want to make permanent changes to this value.
+#ifdef TARGET_ARM64_MS_ABI
+# define STATIC_CHAIN_REGNUM R17_REGNUM
+#else
+# define STATIC_CHAIN_REGNUM R18_REGNUM
+#endif
If we went the enum way, we'd want something like
#define STATIC_CHAIN_REGNUM (calling_abi == AARCH64_CALLING_ABI_MS ? R17_REGNUM : R18_REGNUM)
R.
On 21/02/2024 18:30, Evgeny Karpov wrote:
>
+ tm_defines="${tm_defines} TARGET_ARM64_MS_ABI=1"
I missed this on first reading...
The GCC port name uses AARCH64, please use that internally rather than other names. The only time when we should be using ARM64 is when it's needed for compatibility with other compilers and that doesn't apply here AFAICT.
R.
Hi Richard,
Thanks for the review!
TARGET_ARM64_MS_ABI refers to the official Microsoft ARM64 ABI naming used for the target.
If AARCH64 is a more preferred name, it will be changed in PATCH v2.
https://learn.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=msvc-170
Regards,
Evgeny
-----Original Message-----
Thursday, February 22, 2024 2:11 PM
Richard Earnshaw (lists) wrote:
On 21/02/2024 18:30, Evgeny Karpov wrote:
>
+ tm_defines="${tm_defines} TARGET_ARM64_MS_ABI=1"
I missed this on first reading...
The GCC port name uses AARCH64, please use that internally rather than other names. The only time when we should be using ARM64 is when it's needed for compatibility with other compilers and that doesn't apply here AFAICT.
R.
On Thu, Feb 22, 2024 at 3:56 AM Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote: > > On 21/02/2024 18:30, Evgeny Karpov wrote: > > > +/* X18 reserved for the TEB on Windows. */ > +#ifdef TARGET_ARM64_MS_ABI > +# define FIXED_X18 1 > +# define CALL_USED_X18 0 > +#else > +# define FIXED_X18 0 > +# define CALL_USED_X18 1 > +#endif > > I'm not overly keen on ifdefs like this (and the one below), it can get quite confusing if we have to support more than a couple of ABIs. Perhaps we could create a couple of new headers, one for the EABI (which all existing targets would then need to include) and one for the MS ABI. Then the mingw port would use that instead of the EABI header. > > An alternative is to make all this dynamic, based on the setting of the aarch64_calling_abi enum and to make the adjustments in aarch64_conditional_register_usage. Dynamically might be needed also if we want to support ms_abi attribute and/or -mabi=ms to support the wine folks. Thanks, Andrew Pinski > > +# define CALL_USED_X18 0 > > Is that really correct? If the register is really reserved, but some code modifies it anyway, this will cause the compiler to restore the old value at the end of a function; generally, for a reserved register, code that knows what it's doing would want to make permanent changes to this value. > > +#ifdef TARGET_ARM64_MS_ABI > +# define STATIC_CHAIN_REGNUM R17_REGNUM > +#else > +# define STATIC_CHAIN_REGNUM R18_REGNUM > +#endif > > If we went the enum way, we'd want something like > > #define STATIC_CHAIN_REGNUM (calling_abi == AARCH64_CALLING_ABI_MS ? R17_REGNUM : R18_REGNUM) > > R.
> On 22 Feb 2024, at 17:45, Andrew Pinski <pinskia@gmail.com> wrote: > > On Thu, Feb 22, 2024 at 3:56 AM Richard Earnshaw (lists) > <Richard.Earnshaw@arm.com> wrote: >> >> On 21/02/2024 18:30, Evgeny Karpov wrote: >>> >> +/* X18 reserved for the TEB on Windows. */ >> +#ifdef TARGET_ARM64_MS_ABI >> +# define FIXED_X18 1 >> +# define CALL_USED_X18 0 >> +#else >> +# define FIXED_X18 0 >> +# define CALL_USED_X18 1 >> +#endif >> >> I'm not overly keen on ifdefs like this (and the one below), it can get quite confusing if we have to support more than a couple of ABIs. Perhaps we could create a couple of new headers, one for the EABI (which all existing targets would then need to include) and one for the MS ABI. Then the mingw port would use that instead of the EABI header. >> >> An alternative is to make all this dynamic, based on the setting of the aarch64_calling_abi enum and to make the adjustments in aarch64_conditional_register_usage. > > Dynamically might be needed also if we want to support ms_abi > attribute and/or -mabi=ms to support the wine folks. X18 is also reserved on Darwin - in my current branch I have it non-dynamic too. Iain > > Thanks, > Andrew Pinski > >> >> +# define CALL_USED_X18 0 >> >> Is that really correct? If the register is really reserved, but some code modifies it anyway, this will cause the compiler to restore the old value at the end of a function; generally, for a reserved register, code that knows what it's doing would want to make permanent changes to this value. >> >> +#ifdef TARGET_ARM64_MS_ABI >> +# define STATIC_CHAIN_REGNUM R17_REGNUM >> +#else >> +# define STATIC_CHAIN_REGNUM R18_REGNUM >> +#endif >> >> If we went the enum way, we'd want something like >> >> #define STATIC_CHAIN_REGNUM (calling_abi == AARCH64_CALLING_ABI_MS ? R17_REGNUM : R18_REGNUM) >> >> R.
On 22.02.2024 18:45, Andrew Pinski wrote: > On Thu, Feb 22, 2024 at 3:56 AM Richard Earnshaw (lists) > <Richard.Earnshaw@arm.com> wrote: >> On 21/02/2024 18:30, Evgeny Karpov wrote: >> +/* X18 reserved for the TEB on Windows. */ >> +#ifdef TARGET_ARM64_MS_ABI >> +# define FIXED_X18 1 >> +# define CALL_USED_X18 0 >> +#else >> +# define FIXED_X18 0 >> +# define CALL_USED_X18 1 >> +#endif >> >> I'm not overly keen on ifdefs like this (and the one below), it can get quite confusing if we have to support more than a couple of ABIs. Perhaps we could create a couple of new headers, one for the EABI (which all existing targets would then need to include) and one for the MS ABI. Then the mingw port would use that instead of the EABI header. >> >> An alternative is to make all this dynamic, based on the setting of the aarch64_calling_abi enum and to make the adjustments in aarch64_conditional_register_usage. > Dynamically might be needed also if we want to support ms_abi > attribute and/or -mabi=ms to support the wine folks. Wine no longer needs ms_abi, it was needed for PE-in-ELF modules in the past. We use use proper PE files now, so we need a cross compiler, but no special attributes. aarch64-w64-mingw32 is already well supported by Wine when using llvm-mingw, so as soon as GCC properly supports the ABI, Wine should just work with it, in theory. I didn't try it, but I don't see things like vararg support in this patchset nor in the repo, so I assume it won't work yet. Thanks for the work! Jacek
"Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> writes: > On 21/02/2024 18:30, Evgeny Karpov wrote: >> > +/* X18 reserved for the TEB on Windows. */ > +#ifdef TARGET_ARM64_MS_ABI > +# define FIXED_X18 1 > +# define CALL_USED_X18 0 > +#else > +# define FIXED_X18 0 > +# define CALL_USED_X18 1 > +#endif > > I'm not overly keen on ifdefs like this (and the one below), it can get quite confusing if we have to support more than a couple of ABIs. Perhaps we could create a couple of new headers, one for the EABI (which all existing targets would then need to include) and one for the MS ABI. Then the mingw port would use that instead of the EABI header. > > An alternative is to make all this dynamic, based on the setting of the aarch64_calling_abi enum and to make the adjustments in aarch64_conditional_register_usage. Agreed FWIW. > +# define CALL_USED_X18 0 > > Is that really correct? If the register is really reserved, but some code modifies it anyway, this will cause the compiler to restore the old value at the end of a function; generally, for a reserved register, code that knows what it's doing would want to make permanent changes to this value. I don't think it would do that for fixed registers. For those this is more whether calls are allowed to change the value of x18 or whether x18 is supposed to remain fixed (e.g. set at the start of the thread and not changed thereafter). How does the MS ABI use this register? Same question for Darwin I suppose. Thanks, Richard > > +#ifdef TARGET_ARM64_MS_ABI > +# define STATIC_CHAIN_REGNUM R17_REGNUM > +#else > +# define STATIC_CHAIN_REGNUM R18_REGNUM > +#endif > > If we went the enum way, we'd want something like > > #define STATIC_CHAIN_REGNUM (calling_abi == AARCH64_CALLING_ABI_MS ? R17_REGNUM : R18_REGNUM) > > R.
This change has been refactored based on the review and will be included in v2.
Original aarch64.h will remain unchanged, and the required definition for MS ABI will be implemented in aarch64-abi-ms.h. The reference to this file will be added to config.gcc.
This change has been verified, and it appears to work correctly.
The X18 register should not be used in any case.
The call used flag should be set to 0 for the X18 register to prevent its use in function calls.
The same applies to the static chain. The X17 register will be used instead of X18.
Regards,
Evgeny
gcc/config.gcc
@@ -1263,7 +1263,8 @@
aarch64-*-mingw*)
+ tm_file="${tm_file} aarch64/aarch64-abi-ms.h"
gcc/config/aarch64/aarch64-abi-ms.h
/* Copyright */
/* X18 reserved for the TEB on Windows. */
#undef FIXED_REGISTERS
#define FIXED_REGISTERS \
{ \
0, 0, 0, 0, 0, 0, 0, 0, /* R0 - R7 */ \
0, 0, 0, 0, 0, 0, 0, 0, /* R8 - R15 */ \
0, 0, 1, 0, 0, 0, 0, 0, /* R16 - R23. */ \
0, 0, 0, 0, 0, 1, 0, 1, /* R24 - R30, SP */ \
0, 0, 0, 0, 0, 0, 0, 0, /* V0 - V7 */ \
0, 0, 0, 0, 0, 0, 0, 0, /* V8 - V15 */ \
0, 0, 0, 0, 0, 0, 0, 0, /* V16 - V23 */ \
0, 0, 0, 0, 0, 0, 0, 0, /* V24 - V31 */ \
1, 1, 1, 1, /* SFP, AP, CC, VG */ \
0, 0, 0, 0, 0, 0, 0, 0, /* P0 - P7 */ \
0, 0, 0, 0, 0, 0, 0, 0, /* P8 - P15 */ \
1, 1, /* FFR and FFRT */ \
1, 1, 1, 1, 1, 1, 1, 1 /* Fake registers */ \
}
#undef CALL_REALLY_USED_REGISTERS
#define CALL_REALLY_USED_REGISTERS \
{ \
1, 1, 1, 1, 1, 1, 1, 1, /* R0 - R7 */ \
1, 1, 1, 1, 1, 1, 1, 1, /* R8 - R15 */ \
1, 1, 0, 0, 0, 0, 0, 0, /* R16 - R23. */ \
0, 0, 0, 0, 0, 1, 1, 1, /* R24 - R30, SP */ \
1, 1, 1, 1, 1, 1, 1, 1, /* V0 - V7 */ \
0, 0, 0, 0, 0, 0, 0, 0, /* V8 - V15 */ \
1, 1, 1, 1, 1, 1, 1, 1, /* V16 - V23 */ \
1, 1, 1, 1, 1, 1, 1, 1, /* V24 - V31 */ \
1, 1, 1, 0, /* SFP, AP, CC, VG */ \
1, 1, 1, 1, 1, 1, 1, 1, /* P0 - P7 */ \
1, 1, 1, 1, 1, 1, 1, 1, /* P8 - P15 */ \
1, 1, /* FFR and FFRT */ \
0, 0, 0, 0, 0, 0, 0, 0 /* Fake registers */ \
}
#undef STATIC_CHAIN_REGNUM
#define STATIC_CHAIN_REGNUM R17_REGNUM
-----Original Message-----
Thursday, February 22, 2024 12:55 PM
Richard Earnshaw (lists) wrote:
+/* X18 reserved for the TEB on Windows. */ #ifdef TARGET_ARM64_MS_ABI
+# define FIXED_X18 1 # define CALL_USED_X18 0 #else # define FIXED_X18
+0 # define CALL_USED_X18 1 #endif
I'm not overly keen on ifdefs like this (and the one below), it can get quite confusing if we have to support more than a couple of ABIs. Perhaps we could create a couple of new headers, one for the EABI (which all existing targets would then need to include) and one for the MS ABI. Then the mingw port would use that instead of the EABI header.
An alternative is to make all this dynamic, based on the setting of the aarch64_calling_abi enum and to make the adjustments in aarch64_conditional_register_usage.
+# define CALL_USED_X18 0
Is that really correct? If the register is really reserved, but some code modifies it anyway, this will cause the compiler to restore the old value at the end of a function; generally, for a reserved register, code that knows what it's doing would want to make permanent changes to this value.
+#ifdef TARGET_ARM64_MS_ABI
+# define STATIC_CHAIN_REGNUM R17_REGNUM
+#else
+# define STATIC_CHAIN_REGNUM R18_REGNUM
+#endif
If we went the enum way, we'd want something like
#define STATIC_CHAIN_REGNUM (calling_abi == AARCH64_CALLING_ABI_MS ? R17_REGNUM : R18_REGNUM)
R.
Thank you Jacek for clarifying Wine's needs for ms_abi! Work on vararg support is planned. Regards, Evgeny -----Original Message----- Friday, February 23, 2024 5:10 PM Jacek Caban wrote: > Dynamically might be needed also if we want to support ms_abi > attribute and/or -mabi=ms to support the wine folks. Wine no longer needs ms_abi, it was needed for PE-in-ELF modules in the past. We use use proper PE files now, so we need a cross compiler, but no special attributes. aarch64-w64-mingw32 is already well supported by Wine when using llvm-mingw, so as soon as GCC properly supports the ABI, Wine should just work with it, in theory. I didn't try it, but I don't see things like vararg support in this patchset nor in the repo, so I assume it won't work yet. Thanks for the work! Jacek
In user mode on Windows, it points to TEB (Thread Environment Block). more information here https://learn.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=msvc-170#integer-registers https://learn.microsoft.com/en-us/windows/win32/api/winternl/ns-winternl-teb Regards, Evgeny -----Original Message----- Friday, February 23, 2024 5:56 PM Richard Sandiford wrote: How does the MS ABI use this register? Same question for Darwin I suppose. Thanks, Richard
diff --git a/gcc/config.gcc b/gcc/config.gcc index 092a091595d..2a9e4c44f50 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -1276,6 +1276,7 @@ aarch64*-*-mingw*) default_use_cxa_atexit=yes need_64bit_isa=yes user_headers_inc_next_post="${user_headers_inc_next_post} float.h" + tm_defines="${tm_defines} TARGET_ARM64_MS_ABI=1" ;; aarch64*-wrs-vxworks*) tm_file="${tm_file} elfos.h aarch64/aarch64-elf.h" diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 45e901cda64..36916e7a97d 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -536,11 +536,20 @@ constexpr auto AARCH64_FL_DEFAULT_ISA_MODE = AARCH64_FL_SM_OFF; register. GCC internally uses the poly_int variable aarch64_sve_vg instead. */ +/* X18 reserved for the TEB on Windows. */ +#ifdef TARGET_ARM64_MS_ABI +# define FIXED_X18 1 +# define CALL_USED_X18 0 +#else +# define FIXED_X18 0 +# define CALL_USED_X18 1 +#endif + #define FIXED_REGISTERS \ { \ 0, 0, 0, 0, 0, 0, 0, 0, /* R0 - R7 */ \ 0, 0, 0, 0, 0, 0, 0, 0, /* R8 - R15 */ \ - 0, 0, 0, 0, 0, 0, 0, 0, /* R16 - R23 */ \ + 0, 0, FIXED_X18, 0, 0, 0, 0, 0, /* R16 - R23. */ \ 0, 0, 0, 0, 0, 1, 0, 1, /* R24 - R30, SP */ \ 0, 0, 0, 0, 0, 0, 0, 0, /* V0 - V7 */ \ 0, 0, 0, 0, 0, 0, 0, 0, /* V8 - V15 */ \ @@ -564,7 +573,7 @@ constexpr auto AARCH64_FL_DEFAULT_ISA_MODE = AARCH64_FL_SM_OFF; { \ 1, 1, 1, 1, 1, 1, 1, 1, /* R0 - R7 */ \ 1, 1, 1, 1, 1, 1, 1, 1, /* R8 - R15 */ \ - 1, 1, 1, 0, 0, 0, 0, 0, /* R16 - R23 */ \ + 1, 1, CALL_USED_X18, 0, 0, 0, 0, 0, /* R16 - R23. */ \ 0, 0, 0, 0, 0, 1, 1, 1, /* R24 - R30, SP */ \ 1, 1, 1, 1, 1, 1, 1, 1, /* V0 - V7 */ \ 0, 0, 0, 0, 0, 0, 0, 0, /* V8 - V15 */ \ @@ -642,7 +651,11 @@ constexpr auto AARCH64_FL_DEFAULT_ISA_MODE = AARCH64_FL_SM_OFF; uses alloca. */ #define EXIT_IGNORE_STACK (cfun->calls_alloca) -#define STATIC_CHAIN_REGNUM R18_REGNUM +#ifdef TARGET_ARM64_MS_ABI +# define STATIC_CHAIN_REGNUM R17_REGNUM +#else +# define STATIC_CHAIN_REGNUM R18_REGNUM +#endif #define HARD_FRAME_POINTER_REGNUM R29_REGNUM #define FRAME_POINTER_REGNUM SFP_REGNUM #define STACK_POINTER_REGNUM SP_REGNUM
From 72ca3f49e3eef9b18946b8d4e77019c1441e1a97 Mon Sep 17 00:00:00 2001 From: Zac Walker <zacwalker@microsoft.com> Date: Tue, 20 Feb 2024 15:30:33 +0100 Subject: [PATCH v1 03/13] aarch64: Mark x18 register as a fixed register for MS ABI Define the MS ABI for aarch64-w64-mingw32. Adjust FIXED_REGISTERS and STATIC_CHAIN_REGNUM for different ABIs. The X18 register is reserved on Windows for the TEB. gcc/ChangeLog: * config.gcc: Define TARGET_ARM64_MS_ABI when Arm64 MS ABI is used. * config/aarch64/aarch64.h (FIXED_X18): Define if X18 regsiter is fixed. (CALL_USED_X18): Define if X18 register is call used. (FIXED_REGISTERS): Adjust FIXED_REGISTERS for different ABIs. (STATIC_CHAIN_REGNUM): Define STATIC_CHAIN_REGNUM acording to ABI. --- gcc/config.gcc | 1 + gcc/config/aarch64/aarch64.h | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-)