| Message ID | 20230603112532.3264658-1-xry111@xry111.site |
|---|---|
| State | New |
| Headers | show |
| Series | libatomic: x86_64: Always try ifunc | expand |
On 3 June 2023 13:25:32 CEST, Xi Ruoyao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: >There seems no good way to check if the CPU is Intel or AMD from >the built-in macros (maybe we can check every known model like __skylake, >__bdver2, ..., but it will be very error-prune and require an update >whenever we add the support for a new x86 model). The best thing we can >do seems "always try ifunc" here. IIRC there is __builtin_cpu_is (after initialisation) -- A couple of days ago, we wondered if it would be handy to lower that even in fortran without going through C, so i am pretty sure I don't make that up.. ;-) Just a thought,
On Sat, 2023-06-03 at 14:53 +0200, Bernhard Reutner-Fischer wrote: > On 3 June 2023 13:25:32 CEST, Xi Ruoyao via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > There seems no good way to check if the CPU is Intel or AMD from > > the built-in macros (maybe we can check every known model like > > __skylake, > > __bdver2, ..., but it will be very error-prune and require an update > > whenever we add the support for a new x86 model). The best thing we > > can > > do seems "always try ifunc" here. > > IIRC there is __builtin_cpu_is (after initialisation) -- A couple of > days ago, we wondered if it would be handy to lower that even in > fortran without going through C, so i am pretty sure I don't make that > up.. ;-) Unfortunately __builtin_cpu_is performs CPU detection on runtime, not compile time.
On 3 June 2023 15:46:02 CEST, Xi Ruoyao <xry111@xry111.site> wrote: >Unfortunately __builtin_cpu_is performs CPU detection on runtime, not >compile time. Right, you were talking about configure, sorry.
Ping (in hopes that someone can review before the weekend). On Sat, 2023-06-03 at 19:25 +0800, Xi Ruoyao wrote: > We used to skip ifunc check when CX16 is available. But now we use > CX16+AVX+Intel/AMD for the "perfect" 16b load implementation, so CX16 > alone is not a sufficient reason not to use ifunc (see PR104688). > > This causes a subtle and annoying issue: when GCC is built with a > higher -march= setting in CFLAGS_FOR_TARGET, ifunc is disabled and > the worst (locked) implementation of __atomic_load_16 is always used. > > There seems no good way to check if the CPU is Intel or AMD from > the built-in macros (maybe we can check every known model like > __skylake, > __bdver2, ..., but it will be very error-prune and require an update > whenever we add the support for a new x86 model). The best thing we > can > do seems "always try ifunc" here. > > Bootstrapped and tested on x86_64-linux-gnu. Ok for trunk? > > libatomic/ChangeLog: > > * configure.tgt: For x86_64, always set try_ifunc=yes. > --- > libatomic/configure.tgt | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt > index a92ae9e8309..39dd5686f2e 100644 > --- a/libatomic/configure.tgt > +++ b/libatomic/configure.tgt > @@ -100,9 +100,7 @@ EOF > fi > cat > conftestx.c <<EOF > #ifdef __x86_64__ > -#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 > -#error need -mcx16 > -#endif > +#error ifunc is always wanted for 16B atomic load > #else > #ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 > #error need -march=i686
On Fri, Jun 09, 2023 at 08:37:20PM +0800, Xi Ruoyao wrote: > Ping (in hopes that someone can review before the weekend). > > On Sat, 2023-06-03 at 19:25 +0800, Xi Ruoyao wrote: > > We used to skip ifunc check when CX16 is available. But now we use > > CX16+AVX+Intel/AMD for the "perfect" 16b load implementation, so CX16 > > alone is not a sufficient reason not to use ifunc (see PR104688). > > > > This causes a subtle and annoying issue: when GCC is built with a > > higher -march= setting in CFLAGS_FOR_TARGET, ifunc is disabled and > > the worst (locked) implementation of __atomic_load_16 is always used. > > > > There seems no good way to check if the CPU is Intel or AMD from > > the built-in macros (maybe we can check every known model like > > __skylake, > > __bdver2, ..., but it will be very error-prune and require an update > > whenever we add the support for a new x86 model). The best thing we > > can > > do seems "always try ifunc" here. > > > > Bootstrapped and tested on x86_64-linux-gnu. Ok for trunk? > > > > libatomic/ChangeLog: > > > > * configure.tgt: For x86_64, always set try_ifunc=yes. Ok, thanks. Jakub
diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt index a92ae9e8309..39dd5686f2e 100644 --- a/libatomic/configure.tgt +++ b/libatomic/configure.tgt @@ -100,9 +100,7 @@ EOF fi cat > conftestx.c <<EOF #ifdef __x86_64__ -#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 -#error need -mcx16 -#endif +#error ifunc is always wanted for 16B atomic load #else #ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 #error need -march=i686