Re: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
this patch added double notes on crt*.o and lse derived objects. (which does not seem to cause build break but some linkers may not like it) after #include "aarch64-asm.h" all gnu-stack and gnu-property related stuff should be removed since the header takes care of it.
Re: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
The 01/15/2024 17:21, Radek Barton wrote: v4-0001-Ifdef-.hidden-.type-and-.size-pseudo-ops-for-aarc.patch Description: v4-0001-Ifdef-.hidden-.type-and-.size-pseudo-ops-for-aarc.patch
Re: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
Radek Barton writes: > Hello Richard. > > Thank you for your suggestion. I am sending a patch update according to it. > >> How about avoiding the clash by using the names HIDDEN, SYMBOL_TYPE and >> SYMBOL_SIZE, with SYMBOL_TYPE taking the symbol type as argument? > > Yes, unless the symbol is explicitly exported using `__declspec(dllexport)`, > it will be effectively hidden. > >> What's the practical effect of not marking the symbols as hidden on >> mingw32? Will they still be local to the DLL/EXE, since they haven't >>been explicitly exported? (Sorry for the probably dumb question.) Thanks for the updated patch and sorry for the slow reply -- I was away last week. I've now pushed the patch after testing on aarch64-linux-gnu. Richard
Re: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
Are there any further comments or suggestions, please? What needs to be done to merge this change? (Note we don't have merge rights). Thank you. Radek
Re: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
Wrong attachment, sorry. v4-0001-Ifdef-.hidden-.type-and-.size-pseudo-ops-for-aarc.patch Description: v4-0001-Ifdef-.hidden-.type-and-.size-pseudo-ops-for-aarc.patch
Re: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
Hello Richard. Thank you for your suggestion. I am sending a patch update according to it. > How about avoiding the clash by using the names HIDDEN, SYMBOL_TYPE and > SYMBOL_SIZE, with SYMBOL_TYPE taking the symbol type as argument? Yes, unless the symbol is explicitly exported using `__declspec(dllexport)`, it will be effectively hidden. > What's the practical effect of not marking the symbols as hidden on > mingw32? Will they still be local to the DLL/EXE, since they haven't >been explicitly exported? (Sorry for the probably dumb question.) Best regards, Radek Bartoň v4-0001-Ifdef-.hidden-.type-and-.size-pseudo-ops-for-aarc.patch Description: v4-0001-Ifdef-.hidden-.type-and-.size-pseudo-ops-for-aarc.patch
Re: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
Radek Barton writes: > Hello. > > I forgot to add the target maintainers to the CC. My apologies for that. > > Furthermore, I am adding also relevant changes in > `libgcc/config/aarch64/lse.S` file to the patch. Originally we wanted to > submit those changes separately but after the feedback from Andrew Pinski, it > makes sense to add them here. I needed to rename `HIDDEN`, `TYPE`, and `SIZE` > macros to `HIDDEN_PO`, `TYPE_PO`, and `SIZE_PO` (pseudo-op) because there is > a collision with other macro named `SIZE` in the `lse.S` file. How about avoiding the clash by using the names HIDDEN, SYMBOL_TYPE and SYMBOL_SIZE, with SYMBOL_TYPE taking the symbol type as argument? What's the practical effect of not marking the symbols as hidden on mingw32? Will they still be local to the DLL/EXE, since they haven't been explicitly exported? (Sorry for the probably dumb question.) SME support for mingw32 will be limited until __aarch64_have_sme.c is ported to Windows. Until then, __aarch64_have_sme will just reflect the --with-cpu/arch default. But that obviously doesn't need to be fixed at the same time, just saying for the record. Thanks, Richard > > Best regards, > > Radek > > From eb30feb218f122db8d8d8970e7e1d6d1514ab6c4 Mon Sep 17 00:00:00 2001 > In-Reply-To: > > References: > > From: Zac Walker > Date: Wed, 3 Jan 2024 20:21:04 +0100 > Subject: [PATCH v3] Ifdef `.hidden`, `.type`, and `.size` pseudo-ops for > `aarch64-w64-mingw32` target > Cc: Andrew Pinski , > Richard Sandiford , > Jonathan Yong <10wa...@gmail.com> > > Recent change > (https://gcc.gnu.org/pipermail/gcc-cvs/2023-December/394915.html) added a > generic SME support using `.hidden`, `.type`, and ``.size` pseudo-ops in the > assembly sources, `aarch64-w64-mingw32` does not support the pseudo-ops > though. This patch wraps usage of those pseudo-ops using macros and ifdefs > them for `__ELF__` define. > --- > libgcc/config/aarch64/__arm_sme_state.S | 2 +- > libgcc/config/aarch64/__arm_tpidr2_save.S | 4 ++-- > libgcc/config/aarch64/__arm_za_disable.S | 6 +++--- > libgcc/config/aarch64/aarch64-asm.h | 14 -- > libgcc/config/aarch64/crti.S | 12 > libgcc/config/aarch64/lse.S | 9 + > 6 files changed, 27 insertions(+), 20 deletions(-) > > diff --git a/libgcc/config/aarch64/__arm_sme_state.S > b/libgcc/config/aarch64/__arm_sme_state.S > index 0da9b585b6c..8658da5dfa7 100644 > --- a/libgcc/config/aarch64/__arm_sme_state.S > +++ b/libgcc/config/aarch64/__arm_sme_state.S > @@ -30,7 +30,7 @@ > - Takes no argument. > - Returns SME state in x0 and TPIDR2_EL0 in x1. */ > > -.hidden __aarch64_have_sme > +HIDDEN_PO (__aarch64_have_sme) > > variant_pcs (__arm_sme_state) > > diff --git a/libgcc/config/aarch64/__arm_tpidr2_save.S > b/libgcc/config/aarch64/__arm_tpidr2_save.S > index 9135cba1ddb..739694ed189 100644 > --- a/libgcc/config/aarch64/__arm_tpidr2_save.S > +++ b/libgcc/config/aarch64/__arm_tpidr2_save.S > @@ -31,7 +31,7 @@ > - Does not return a value. > - Can abort on failure (then registers are not preserved). */ > > -.hidden __aarch64_have_sme > +HIDDEN_PO (__aarch64_have_sme) > > variant_pcs (__arm_tpidr2_save) > > @@ -97,5 +97,5 @@ END (__arm_tpidr2_save) > > /* Hidden alias used by __arm_za_disable. */ > .global __libgcc_arm_tpidr2_save > -.hidden __libgcc_arm_tpidr2_save > +HIDDEN_PO (__libgcc_arm_tpidr2_save) > .set __libgcc_arm_tpidr2_save, __arm_tpidr2_save > diff --git a/libgcc/config/aarch64/__arm_za_disable.S > b/libgcc/config/aarch64/__arm_za_disable.S > index 5785a959e22..95eae3ea958 100644 > --- a/libgcc/config/aarch64/__arm_za_disable.S > +++ b/libgcc/config/aarch64/__arm_za_disable.S > @@ -31,9 +31,9 @@ > - Does not return a value. > - Can abort on failure (then registers are not preserved). */ > > -.hidden __aarch64_have_sme > +HIDDEN_PO (__aarch64_have_sme) > > -.hidden __libgcc_arm_tpidr2_save > +HIDDEN_PO (__libgcc_arm_tpidr2_save) > > variant_pcs (__arm_za_disable) > > @@ -66,5 +66,5 @@ END (__arm_za_disable) > > /* Hidden alias used by the unwinder. */ > .global __libgcc_arm_za_disable > -.hidden __libgcc_arm_za_disable > +HIDDEN_PO (__libgcc_arm_za_disable) > .set __libgcc_arm_za_disable, __arm_za_disable > diff --git a/libgcc/config/aarch64/aarch64-asm.h > b/libgcc/config/aarch64/aarch64-asm.h > index 24568429b5c..dbb81b4be6b 100644 > --- a/libgcc/config/aarch64/aarch64-asm.h > +++ b/libgcc/config/aarch64/aarch64-asm.h > @@ -58,6 +58,16 @@ > # define AUTIASP > #endif > > +#ifdef __ELF__ > +#define TYPE_PO(x) .type x,function > +#define HIDDEN_PO(x) .hidden x > +#define SIZE_PO(x) .size x, .-x > +#else > +#define TYPE_PO(x) > +#define HIDDEN_PO(x) > +#define SIZE_PO(x) > +#endif > + > /* Add a NT_GNU_PROPERTY_TYPE_0 note. */ > #define GNU_PROPERTY(type, value)\ >.section .note.gnu.property, "a"; \ > @@ -85,7 +95,7 @@ GNU_PROPERTY (FEATURE_1_AND, BT
Re: [EXTERNAL] Re: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
Originally we've used `!__MINGW64__` but changed it to `__ELF__` upon feedback received. Should I change it back to `!__MINGW64__`? Or introduce '__COFF__' and then use `!__COFF__`? What would be the minimal acceptable change? we are currently probably not able to provide that generic solution as has Iain Sandoe implied. Note that we have moved the pseudo-ops wrapper macros to the `libgcc/config/aarch64/aarch64-asm.h` file already. Thank you all for your valuable feedback. Radek
Re: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
> On 10 Jan 2024, at 09:02, Iain Sandoe wrote: >> On 10 Jan 2024, at 08:49, Jonathan Yong <10wa...@gmail.com> wrote: >> >> On 1/9/24 19:37, Radek Barton wrote: >>> Hello. >>> I forgot to add the target maintainers to the CC. My apologies for that. >>> Furthermore, I am adding also relevant changes in >>> `libgcc/config/aarch64/lse.S` file to the patch. Originally we wanted to >>> submit those changes separately but after the feedback from Andrew Pinski, >>> it makes sense to add them here. I needed to rename `HIDDEN`, `TYPE`, and >>> `SIZE` macros to `HIDDEN_PO`, `TYPE_PO`, and `SIZE_PO` (pseudo-op) because >>> there is a collision with other macro named `SIZE` in the `lse.S` file. >>> Best regards, >>> Radek >> >> Looks fine to me, but is __ELF__ correct? I am not familiar with pseudo-ops, >> OK if it is ELF specific when PE is targeted. >> > > I suspect that, the end, we really need to generalize this so that ELF, > XCOFF, Mach-O etc. are handled. In other places in the tree, typically an > “asm.h” (or similar name) is included which contains macros that adjust: > > global symbol > local symbol > type > size > > (and sometimes .cfi_-related) > > Then the asm sources are adjusted to use those macros throughout, which > means that they build correctly for the different object file formats. > > You should be able to find a suitable example in other ports which could be > updated to cater for aarch64-specific cases. duh, I was not looking hard enough - it seems that there is already such a file libgcc/config/aarch64/aarch64-asm.h It has just not been used in the SME stuff. Iain
Re: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
> On 10 Jan 2024, at 08:49, Jonathan Yong <10wa...@gmail.com> wrote: > > On 1/9/24 19:37, Radek Barton wrote: >> Hello. >> I forgot to add the target maintainers to the CC. My apologies for that. >> Furthermore, I am adding also relevant changes in >> `libgcc/config/aarch64/lse.S` file to the patch. Originally we wanted to >> submit those changes separately but after the feedback from Andrew Pinski, >> it makes sense to add them here. I needed to rename `HIDDEN`, `TYPE`, and >> `SIZE` macros to `HIDDEN_PO`, `TYPE_PO`, and `SIZE_PO` (pseudo-op) because >> there is a collision with other macro named `SIZE` in the `lse.S` file. >> Best regards, >> Radek > > Looks fine to me, but is __ELF__ correct? I am not familiar with pseudo-ops, > OK if it is ELF specific when PE is targeted. > I suspect that, the end, we really need to generalize this so that ELF, XCOFF, Mach-O etc. are handled. In other places in the tree, typically an “asm.h” (or similar name) is included which contains macros that adjust: global symbol local symbol type size (and sometimes .cfi_-related) Then the asm sources are adjusted to use those macros throughout, which means that they build correctly for the different object file formats. You should be able to find a suitable example in other ports which could be updated to cater for aarch64-specific cases. 0.02GBP only, ( I do not have cycles to tackle this myself right now, although I have temporary workarounds in my Darwin branch) Iain
Re: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
On 1/9/24 19:37, Radek Barton wrote: Hello. I forgot to add the target maintainers to the CC. My apologies for that. Furthermore, I am adding also relevant changes in `libgcc/config/aarch64/lse.S` file to the patch. Originally we wanted to submit those changes separately but after the feedback from Andrew Pinski, it makes sense to add them here. I needed to rename `HIDDEN`, `TYPE`, and `SIZE` macros to `HIDDEN_PO`, `TYPE_PO`, and `SIZE_PO` (pseudo-op) because there is a collision with other macro named `SIZE` in the `lse.S` file. Best regards, Radek Looks fine to me, but is __ELF__ correct? I am not familiar with pseudo-ops, OK if it is ELF specific when PE is targeted.
Re: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
Hello. I forgot to add the target maintainers to the CC. My apologies for that. Furthermore, I am adding also relevant changes in `libgcc/config/aarch64/lse.S` file to the patch. Originally we wanted to submit those changes separately but after the feedback from Andrew Pinski, it makes sense to add them here. I needed to rename `HIDDEN`, `TYPE`, and `SIZE` macros to `HIDDEN_PO`, `TYPE_PO`, and `SIZE_PO` (pseudo-op) because there is a collision with other macro named `SIZE` in the `lse.S` file. Best regards, Radek v3-0001-Ifdef-.hidden-.type-and-.size-pseudo-ops-for-aarc.patch Description: v3-0001-Ifdef-.hidden-.type-and-.size-pseudo-ops-for-aarc.patch
Re: [EXTERNAL] Re: Fw: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
Hello, Andrew. Thank you for your input. I've updated the "fixing" patch according to your feedback. Please let me know if I understood it correctly. Radek From: Andrew Pinski Sent: Thursday, January 4, 2024 8:11 PM To: Radek Barton ; Andrew Pinski (QUIC) Cc: gcc-patches@gcc.gnu.org Subject: [EXTERNAL] Re: Fw: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target? [You don't often get email from pins...@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] On Thu, Jan 4, 2024 at 5:51 AM Radek Barton wrote: > > Hello, everyone. > > > Our "Arm64 on Windows Ecosystem" team is currently working on adding > aarch64-w64-mingw32 target and we've noticed that recent commit adding SME > support > (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-cvs%2F2023-December%2F394915.html&data=05%7C02%7Cradek.barton%40microsoft.com%7C51df4d9506014407bc8908dc0d58eeac%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638399922842775482%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=5%2FK9lrqVSFn35cxjmhyEnKpiArJEMcOp5BQbAr%2F3r1s%3D&reserved=0<https://gcc.gnu.org/pipermail/gcc-cvs/2023-December/394915.html>) > is using .hidden and .size pseudo-ops that are not supported by this target > yet. We'd like to hear your opinion what would be the most acceptable fix for > the community: > > Wrap the unsupported pseudo-ops using macros and #ifdef them for the target. > The attached 0001-Ifdef-.hidden-and-.size-pseudo-ops-for-aarch64-w64-m.patch > is demonstrating this option. > Move SME related sources to a separate config, t-sme, that won't be included > by the aarch64-w64-mingw32 target config. The attached > 0001-Exclude-SME-feature-from-libgcc-for-aarch64-w64-ming.patch by Evgeny > Karpov is a proposal of this change. > Do you have any other proposal? For the .type issue you should use the following define instead: ``` #ifdef __ELF__ #define TYPE(x) .type x,function #else #define TYPE(x) #endif ``` Which comes directly from config/aarch64/crti.S . HIDDEN should be handled similarly. We really should still have SME support for GCC for windows. Thanks, Andrew Pinski > > > Best regards, > > Radek Bartoň 0001-Ifdef-.hidden-.type-and-.size-pseudo-ops-for-aarch64.patch Description: 0001-Ifdef-.hidden-.type-and-.size-pseudo-ops-for-aarch64.patch
Re: Fw: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
On Thu, Jan 4, 2024 at 5:51 AM Radek Barton wrote: > > Hello, everyone. > > > Our "Arm64 on Windows Ecosystem" team is currently working on adding > aarch64-w64-mingw32 target and we've noticed that recent commit adding SME > support (https://gcc.gnu.org/pipermail/gcc-cvs/2023-December/394915.html) is > using .hidden and .size pseudo-ops that are not supported by this target yet. > We'd like to hear your opinion what would be the most acceptable fix for the > community: > > Wrap the unsupported pseudo-ops using macros and #ifdef them for the target. > The attached 0001-Ifdef-.hidden-and-.size-pseudo-ops-for-aarch64-w64-m.patch > is demonstrating this option. > Move SME related sources to a separate config, t-sme, that won't be included > by the aarch64-w64-mingw32 target config. The attached > 0001-Exclude-SME-feature-from-libgcc-for-aarch64-w64-ming.patch by Evgeny > Karpov is a proposal of this change. > Do you have any other proposal? For the .type issue you should use the following define instead: ``` #ifdef __ELF__ #define TYPE(x) .type x,function #else #define TYPE(x) #endif ``` Which comes directly from config/aarch64/crti.S . HIDDEN should be handled similarly. We really should still have SME support for GCC for windows. Thanks, Andrew Pinski > > > Best regards, > > Radek Bartoň
Fw: [RFC] Either fix or disable SME feature for `aarch64-w64-mingw32` target?
Hello, everyone. Our "Arm64 on Windows Ecosystem" team is currently working on adding aarch64-w64-mingw32 target and we've noticed that recent commit adding SME support (https://gcc.gnu.org/pipermail/gcc-cvs/2023-December/394915.html) is using .hidden and .size pseudo-ops that are not supported by this target yet. We'd like to hear your opinion what would be the most acceptable fix for the community: 1. Wrap the unsupported pseudo-ops using macros and #ifdef them for the target. The attached 0001-Ifdef-.hidden-and-.size-pseudo-ops-for-aarch64-w64-m.patch is demonstrating this option. 2. Move SME related sources to a separate config, t-sme, that won't be included by the aarch64-w64-mingw32 target config. The attached 0001-Exclude-SME-feature-from-libgcc-for-aarch64-w64-ming.patch by Evgeny Karpov is a proposal of this change. 3. Do you have any other proposal? Best regards, Radek Bartoň 0001-Exclude-SME-feature-from-libgcc-for-aarch64-w64-ming.patch Description: 0001-Exclude-SME-feature-from-libgcc-for-aarch64-w64-ming.patch 0001-Ifdef-.hidden-and-.size-pseudo-ops-for-aarch64-w64-m.patch Description: 0001-Ifdef-.hidden-and-.size-pseudo-ops-for-aarch64-w64-m.patch
