* [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 @ 2017-08-07 20:44 Steve Ellcey 2017-08-07 20:46 ` Steve Ellcey 2017-08-25 16:43 ` Szabolcs Nagy 0 siblings, 2 replies; 22+ messages in thread From: Steve Ellcey @ 2017-08-07 20:44 UTC (permalink / raw) To: gcc-patches This patch uses the libatomic IFUNC infrastructure so that aarch64 machines that support the LSE instructions can use them.  Note that aarch64 still isn't enabling IFUNC support by default though I have submitted a patch to do that.  You can enable IFUNC support by configuring with --enable-gnu-indirect-function. Glibc has an environment variable, LD_HWCAP_MASK, that can be used to mask out some or all of the bits returned by getauxval(AT_HWCAP) and ignore certain hardware capabilities.  I enabled this functionality for libatomic by looking at the LD_HWCAP_MASK variable in the IFUNC resolver function.  That way, if I had a system that supported LSE but did not want to use it for some reason, I could set LD_HWCAP_MASK to zero and then the IFUNC selector function would not enable the LSE routines.  I could remove this functionality if we thought it was not appropriate but I think it is useful, both for testing and for end users. Tested on aarch64, OK for checkin? Steve Ellcey sellcey@cavium.com 2017-08-07  Steve Ellcey  <sellcey@cavium.com> * Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and libatomic_la_LIBADD. * config/linux/aarch64/host-config.h: New file. * config/linux/aarch64/init.c: New file. * configure.ac (AC_CHECK_HEADERS): Check for sys/auxv.h. (AC_CHECK_FUNCS): Check for getauxval. (ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds. * configure.tgt (aarch64): Set AARCH and try_ifunc. (aarch64*-*-linux*) Update config_path. * Makefile.in: Regenerate. * auto-config.h.in: Regenerate. * configure: Regenerate. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-08-07 20:44 [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 Steve Ellcey @ 2017-08-07 20:46 ` Steve Ellcey 2017-08-25 4:56 ` Steve Ellcey 2017-08-25 16:43 ` Szabolcs Nagy 1 sibling, 1 reply; 22+ messages in thread From: Steve Ellcey @ 2017-08-07 20:46 UTC (permalink / raw) To: gcc-patches [-- Attachment #1: Type: text/plain, Size: 659 bytes --] It would probably help if I included the patch. Steve Ellcey sellcey@cavium.com 2017-08-07  Steve Ellcey  <sellcey@cavium.com> * Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and libatomic_la_LIBADD. * config/linux/aarch64/host-config.h: New file. * config/linux/aarch64/init.c: New file. * configure.ac (AC_CHECK_HEADERS): Check for sys/auxv.h. (AC_CHECK_FUNCS): Check for getauxval. (ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds. * configure.tgt (aarch64): Set AARCH and try_ifunc. (aarch64*-*-linux*) Update config_path. * Makefile.in: Regenerate. * auto-config.h.in: Regenerate. * configure: Regenerate. [-- Attachment #2: libatomic.patch --] [-- Type: text/x-patch, Size: 5487 bytes --] diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am index d731406..a35df1e 100644 --- a/libatomic/Makefile.am +++ b/libatomic/Makefile.am @@ -122,6 +122,10 @@ libatomic_la_LIBADD = $(foreach s,$(SIZES),$(addsuffix _$(s)_.lo,$(SIZEOBJS))) ## On a target-specific basis, include alternates to be selected by IFUNC. if HAVE_IFUNC +if ARCH_AARCH64_LINUX_LSE +IFUNC_OPTIONS = -mcpu=thunderx2t99 +libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS))) +endif if ARCH_ARM_LINUX IFUNC_OPTIONS = -march=armv7-a -DHAVE_KERNEL64 libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS))) diff --git a/libatomic/config/linux/aarch64/host-config.h b/libatomic/config/linux/aarch64/host-config.h index e69de29..4d0ab96 100644 --- a/libatomic/config/linux/aarch64/host-config.h +++ b/libatomic/config/linux/aarch64/host-config.h @@ -0,0 +1,33 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + + This file is part of the GNU Atomic Library (libatomic). + + Libatomic is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + Libatomic is distributed in the hope that it will be useful, but WITHOUT ANY + WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + FOR A PARTICULAR PURPOSE. See the GNU General Public License for + more details. + + Under Section 7 of GPL version 3, you are granted additional + permissions described in the GCC Runtime Library Exception, version + 3.1, as published by the Free Software Foundation. + + You should have received a copy of the GNU General Public License and + a copy of the GCC Runtime Library Exception along with this program; + see the files COPYING3 and COPYING.RUNTIME respectively. If not, see + <http://www.gnu.org/licenses/>. */ + +#if HAVE_IFUNC + +extern bool libat_have_lse HIDDEN; + +# define IFUNC_COND_1 libat_have_lse +# define IFUNC_NCOND(N) (1) + +#endif /* HAVE_IFUNC */ + +#include_next <host-config.h> diff --git a/libatomic/config/linux/aarch64/init.c b/libatomic/config/linux/aarch64/init.c index e69de29..1b135c2 100644 --- a/libatomic/config/linux/aarch64/init.c +++ b/libatomic/config/linux/aarch64/init.c @@ -0,0 +1,51 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + + This file is part of the GNU Atomic Library (libatomic). + + Libatomic is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + Libatomic is distributed in the hope that it will be useful, but WITHOUT ANY + WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + FOR A PARTICULAR PURPOSE. See the GNU General Public License for + more details. + + Under Section 7 of GPL version 3, you are granted additional + permissions described in the GCC Runtime Library Exception, version + 3.1, as published by the Free Software Foundation. + + You should have received a copy of the GNU General Public License and + a copy of the GCC Runtime Library Exception along with this program; + see the files COPYING3 and COPYING.RUNTIME respectively. If not, see + <http://www.gnu.org/licenses/>. */ + +#include "libatomic_i.h" + +#if HAVE_IFUNC + +#include <stdlib.h> +#ifdef HAVE_SYS_AUXV_H +# include <sys/auxv.h> +#endif + +bool libat_have_lse = false; + +static void __attribute__((constructor)) +init_cpu_revision (void) +{ +#if defined (HAVE_GETAUXVAL) && defined (HWCAP_ATOMICS) + unsigned long i; + char *s; + + i = getauxval (AT_HWCAP); + s = getenv ("LD_HWCAP_MASK"); + if (s) + i = i & strtoul (s, NULL, 10); + if (i & HWCAP_ATOMICS) + libat_have_lse = true; +#endif +} + +#endif /* HAVE_IFUNC */ diff --git a/libatomic/configure.ac b/libatomic/configure.ac index 023f172..2bdc234 100644 --- a/libatomic/configure.ac +++ b/libatomic/configure.ac @@ -171,7 +171,8 @@ CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS" AC_STDC_HEADERS ACX_HEADER_STRING GCC_HEADER_STDINT(gstdint.h) -AC_CHECK_HEADERS([fenv.h]) +AC_CHECK_HEADERS([fenv.h sys/auxv.h]) +AC_CHECK_FUNCS(getauxval) # Check for common type sizes LIBAT_FORALL_MODES([LIBAT_HAVE_INT_MODE]) @@ -247,6 +248,8 @@ AC_SUBST(LIBS) AC_SUBST(SIZES) AM_CONDITIONAL(HAVE_IFUNC, test x$libat_cv_have_ifunc = xyes) +AM_CONDITIONAL(ARCH_AARCH64_LINUX_LSE, + [expr "$config_path" : ".* linux/aarch64 .*" > /dev/null]) AM_CONDITIONAL(ARCH_ARM_LINUX, [expr "$config_path" : ".* linux/arm .*" > /dev/null]) AM_CONDITIONAL(ARCH_I386, diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt index b8af3ab..f70bad5 100644 --- a/libatomic/configure.tgt +++ b/libatomic/configure.tgt @@ -40,6 +40,14 @@ case "${target_cpu}" in riscv*) ARCH=riscv ;; sh*) ARCH=sh ;; + aarch64*) + ARCH=aarch64 + case "${target}" in + aarch64*-*-linux*) + try_ifunc=yes + ;; + esac + ;; arm*) ARCH=arm case "${target}" in @@ -109,6 +117,11 @@ fi # Other system configury case "${target}" in + aarch64*-*-linux*) + # OS support for atomic primitives. + config_path="${config_path} linux/aarch64 posix" + ;; + arm*-*-linux*) # OS support for atomic primitives. config_path="${config_path} linux/arm posix" ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-08-07 20:46 ` Steve Ellcey @ 2017-08-25 4:56 ` Steve Ellcey 0 siblings, 0 replies; 22+ messages in thread From: Steve Ellcey @ 2017-08-25 4:56 UTC (permalink / raw) To: gcc-patches Ping. > 2017-08-07  Steve Ellcey  <sellcey@cavium.com> > > * Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and > libatomic_la_LIBADD. > * config/linux/aarch64/host-config.h: New file. > * config/linux/aarch64/init.c: New file. > * configure.ac (AC_CHECK_HEADERS): Check for sys/auxv.h. > (AC_CHECK_FUNCS): Check for getauxval. > (ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds. > * configure.tgt (aarch64): Set AARCH and try_ifunc. > (aarch64*-*-linux*) Update config_path. > * Makefile.in: Regenerate. > * auto-config.h.in: Regenerate. > * configure: Regenerate. > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-08-07 20:44 [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 Steve Ellcey 2017-08-07 20:46 ` Steve Ellcey @ 2017-08-25 16:43 ` Szabolcs Nagy 2017-08-28 18:40 ` Steve Ellcey 1 sibling, 1 reply; 22+ messages in thread From: Szabolcs Nagy @ 2017-08-25 16:43 UTC (permalink / raw) To: sellcey, gcc-patches; +Cc: nd On 07/08/17 21:44, Steve Ellcey wrote: > This patch uses the libatomic IFUNC infrastructure so that aarch64 > machines that support the LSE instructions can use them. Note that > aarch64 still isn't enabling IFUNC support by default though I have > submitted a patch to do that. You can enable IFUNC support by > configuring with --enable-gnu-indirect-function. > > Glibc has an environment variable, LD_HWCAP_MASK, that can be used to > mask out some or all of the bits returned by getauxval(AT_HWCAP) and > ignore certain hardware capabilities. I enabled this functionality > for libatomic by looking at the LD_HWCAP_MASK variable in the IFUNC > resolver function. That way, if I had a system that supported LSE but > did not want to use it for some reason, I could set LD_HWCAP_MASK to > zero and then the IFUNC selector function would not enable the LSE > routines. I could remove this functionality if we thought it was not > appropriate but I think it is useful, both for testing and for end > users. > the use of ifunc in gcc target libraries was a mistake in my opinion, there are several known bugs in the ifunc design and uclibc/musl/bionic don't plan to support it. but at this point i dont have a better proposal for doing runtime selection of optimal atomics code. however in this patch i don't see why would the ctor run before ifunc resolvers. how does this work on x86_64? (there the different 16byte atomics are not even compatible, so if ifunc resolvers in different dsos return different result because one ran before the ctor, another after then the runtime behaviour is broken. this can happen when one dso is bindnow so ifunc relocation is processed before ctors and another runs resolvers lazily or dlopened later.. but i didnt test it just looks broken) note that aarch64 ifunc resolvers get hwcap as an argument so all this brokenness can be avoided (and if we want to disable hwcap flags then probably glibc should take care of that before passing hwcap to the ifunc resolver). > Tested on aarch64, OK for checkin? > > Steve Ellcey > sellcey@cavium.com > > > > > 2017-08-07 Steve Ellcey <sellcey@cavium.com> > > * Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and > libatomic_la_LIBADD. > * config/linux/aarch64/host-config.h: New file. > * config/linux/aarch64/init.c: New file. > * configure.ac (AC_CHECK_HEADERS): Check for sys/auxv.h. > (AC_CHECK_FUNCS): Check for getauxval. > (ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds. > * configure.tgt (aarch64): Set AARCH and try_ifunc. > (aarch64*-*-linux*) Update config_path. > * Makefile.in: Regenerate. > * auto-config.h.in: Regenerate. > * configure: Regenerate. > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-08-25 16:43 ` Szabolcs Nagy @ 2017-08-28 18:40 ` Steve Ellcey 2017-08-29 11:42 ` Szabolcs Nagy 0 siblings, 1 reply; 22+ messages in thread From: Steve Ellcey @ 2017-08-28 18:40 UTC (permalink / raw) To: Szabolcs Nagy, gcc-patches; +Cc: nd On Fri, 2017-08-25 at 15:37 +0100, Szabolcs Nagy wrote: > the use of ifunc in gcc target libraries was a mistake > in my opinion, there are several known bugs in the ifunc > design and uclibc/musl/bionic don't plan to support it. > but at this point i dont have a better proposal for doing > runtime selection of optimal atomics code. > > however in this patch i don't see why would the ctor run > before ifunc resolvers. how does this work on x86_64? > (there the different 16byte atomics are not even compatible, > so if ifunc resolvers in different dsos return different > result because one ran before the ctor, another after then > the runtime behaviour is broken. this can happen when one > dso is bindnow so ifunc relocation is processed before > ctors and another runs resolvers lazily or dlopened later.. > but i didnt test it just looks broken) I am not sure I followed everything here.  My basic testing all worked, init_cpu_revision got run when libatomic was loaded and then the IFUNC resolver gets called after that when one of the IFUNC atomic functions is called.  init_cpu_revision sets libat_have_lse which, I assume, will not be used by any other libraries.  If other libraries have IFUNCs they would have their own static constructors and set their own variables to be checked by their own IFUNCs.  So I am not sure how one DSO is going to break another DSO. > note that aarch64 ifunc resolvers get hwcap as an argument > so all this brokenness can be avoided (and if we want to > disable hwcap flags then probably glibc should take care > of that before passing hwcap to the ifunc resolver). I looked at the IFUNC memcpy resolver in glibc and it does not look like it gets hwcap as an argument.  When I preprocess everything I see: void *__libc_memcpy_ifunc (void) __asm__ ("__libc_memcpy"); void *__libc_memcpy_ifunc (void) {   uint64_t __attribute__((unused)) midr = _dl_aarch64_cpu_features.midr_el1; *res = ** expression that looks at midr value and returns function pointer **; return res; } __asm__ (".type " "__libc_memcpy" ", %gnu_indirect_function"); extern __typeof (__libc_memcpy) memcpy __attribute__ ((alias ("__libc_memcpy"))); Steve Ellcey sellcey@cavium.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-08-28 18:40 ` Steve Ellcey @ 2017-08-29 11:42 ` Szabolcs Nagy 2017-08-30 18:39 ` Steve Ellcey 2017-08-31 18:55 ` Steve Ellcey 0 siblings, 2 replies; 22+ messages in thread From: Szabolcs Nagy @ 2017-08-29 11:42 UTC (permalink / raw) To: sellcey, gcc-patches; +Cc: nd On 28/08/17 19:25, Steve Ellcey wrote: > On Fri, 2017-08-25 at 15:37 +0100, Szabolcs Nagy wrote: > >> the use of ifunc in gcc target libraries was a mistake >> in my opinion, there are several known bugs in the ifunc >> design and uclibc/musl/bionic don't plan to support it. >> but at this point i dont have a better proposal for doing >> runtime selection of optimal atomics code. >> >> however in this patch i don't see why would the ctor run >> before ifunc resolvers. how does this work on x86_64? >> (there the different 16byte atomics are not even compatible, >> so if ifunc resolvers in different dsos return different >> result because one ran before the ctor, another after then >> the runtime behaviour is broken. this can happen when one >> dso is bindnow so ifunc relocation is processed before >> ctors and another runs resolvers lazily or dlopened later.. >> but i didnt test it just looks broken) > > I am not sure I followed everything here. My basic testing all > worked, init_cpu_revision got run when libatomic was loaded and > then the IFUNC resolver gets called after that when one of the IFUNC > atomic functions is called. init_cpu_revision sets libat_have_lse > which, I assume, will not be used by any other libraries. If other > libraries have IFUNCs they would have their own static constructors and > set their own variables to be checked by their own IFUNCs. So I am > not sure how one DSO is going to break another DSO. > this can only possibly work with lazy binding of the libatomic calls (there are many reasons why that may not be the case starting from static linking to compiling with -fno-plt or setting LD_BIND_NOW at runtime) as i expected this is broken on x86 see the execution test i added to pr 60790: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60790 >> note that aarch64 ifunc resolvers get hwcap as an argument >> so all this brokenness can be avoided (and if we want to >> disable hwcap flags then probably glibc should take care >> of that before passing hwcap to the ifunc resolver). > > I looked at the IFUNC memcpy resolver in glibc and it does not look > like it gets hwcap as an argument. When I preprocess everything I see: > > void *__libc_memcpy_ifunc (void) __asm__ ("__libc_memcpy"); > void *__libc_memcpy_ifunc (void) > { > uint64_t __attribute__((unused)) midr = _dl_aarch64_cpu_features.midr_el1; > *res = ** expression that looks at midr value and returns function pointer **; > return res; > } > __asm__ (".type " "__libc_memcpy" ", %gnu_indirect_function"); > extern __typeof (__libc_memcpy) memcpy __attribute__ ((alias ("__libc_memcpy"))); > it's a general bug that most ifunc users (e.g. in binutils tests) declare the ifunc resolvers incorrectly, the correct prototype is the way the dynamic linker invokes the resolver in sysdeps/aarch64/dl-irel.h: static inline ElfW(Addr) __attribute ((always_inline)) elf_ifunc_invoke (ElfW(Addr) addr) { return ((ElfW(Addr) (*) (unsigned long int)) (addr)) (GLRO(dl_hwcap)); } (that argument should be uint64_t for ilp32, but that's a separate issue) in glibc the hwcap is not used, because it has accesses to cached dispatch info, but in libatomic using the hwcap argument is the right way. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-08-29 11:42 ` Szabolcs Nagy @ 2017-08-30 18:39 ` Steve Ellcey 2017-08-31 18:55 ` Steve Ellcey 1 sibling, 0 replies; 22+ messages in thread From: Steve Ellcey @ 2017-08-30 18:39 UTC (permalink / raw) To: Szabolcs Nagy, gcc-patches; +Cc: nd On Tue, 2017-08-29 at 12:25 +0100, Szabolcs Nagy wrote: >Â > it's a general bug that most ifunc users (e.g. in binutils > tests) declare the ifunc resolvers incorrectly, the correct > prototype is the way the dynamic linker invokes the resolver > in sysdeps/aarch64/dl-irel.h: > > static inline ElfW(Addr) > __attribute ((always_inline)) > elf_ifunc_invoke (ElfW(Addr) addr) > { > Â return ((ElfW(Addr) (*) (unsigned long int)) (addr)) > (GLRO(dl_hwcap)); > } > > (that argument should be uint64_t for ilp32, but that's a > separate issue) > > in glibc the hwcap is not used, because it has accesses to > cached dispatch info, but in libatomic using the hwcap > argument is the right way. OK, I changed my patch to use the argument and it did work but since the type of the argument should be uint64_t instead of 'unsigned long int' for aarch64 I think I will send a patch to libc-alpha first to fix this before sending an updated libatomic patch. Â That way I won't have to update GCC when glibc changes the type. Â I see that different platforms use different types for the resolver argument so I think I will have to update the libatomic configure.tgt script as well to set the resolver arg type when I resbumit the gcc libatomic patch so that we can have the correct prototype in libatomic_i.h. Steve Ellcey sellcey@cavium.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-08-29 11:42 ` Szabolcs Nagy 2017-08-30 18:39 ` Steve Ellcey @ 2017-08-31 18:55 ` Steve Ellcey 2017-09-27 20:35 ` Steve Ellcey 2017-09-28 11:31 ` Szabolcs Nagy 1 sibling, 2 replies; 22+ messages in thread From: Steve Ellcey @ 2017-08-31 18:55 UTC (permalink / raw) To: Szabolcs Nagy, gcc-patches; +Cc: nd [-- Attachment #1: Type: text/plain, Size: 1956 bytes --] On Tue, 2017-08-29 at 12:25 +0100, Szabolcs Nagy wrote: > > in glibc the hwcap is not used, because it has accesses to > cached dispatch info, but in libatomic using the hwcap > argument is the right way. Here is an updated version of the patch to allow aarch64 to use ifuncs in libatomic. The main difference from the last patch is that the library does not access the hwcap value directly but accesses it through the ifunc resolver argument.  That means that we no longer need the init_cpu_revision static constructor to set a flag that the resolver checks, instead the resolver just does a comparision of its incoming argument with HWCAP_ATOMICS. This did mean I had to change the prototype for the resolver functions in libatomic_i.h to have an argument, which is the way glibc calls them.  One complication of this is that the type of the argument can differ between platforms and ABIs so I added code to configure.tgt to set the type.  I used uint64_t for aarch64 and 'long unsigned int' for everything else.  That is not correct for all platforms but at this point no other platforms access the argument so it should not matter.  If and when platforms do need to access it they can change the type if necessary. Steve Ellcey sellcey@cavium.com 2017-08-31  Steve Ellcey  <sellcey@cavium.com> * Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and libatomic_la_LIBADD. * config/linux/aarch64/host-config.h: New file. * configure.ac (HWCAP_TYPE): Define. (AC_CHECK_HEADERS): Check for sys/auxv.h. (AC_CHECK_FUNCS): Check for getauxval. (ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds. * configure.tgt (aarch64): Set AARCH and try_ifunc. (aarch64*-*-linux*) Update config_path. (aarch64*-*-linux*) Set HWCAP_TYPE. * libatomic_i.h (GEN_SELECTOR): Add "HWCAP_TYPE hwcap" argument. * Makefile.in: Regenerate. * auto-config.h.in: Regenerate. * configure: Regenerate. [-- Attachment #2: libatomic.patch --] [-- Type: text/x-patch, Size: 5861 bytes --] diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am index d731406..a35df1e 100644 --- a/libatomic/Makefile.am +++ b/libatomic/Makefile.am @@ -122,6 +122,10 @@ libatomic_la_LIBADD = $(foreach s,$(SIZES),$(addsuffix _$(s)_.lo,$(SIZEOBJS))) ## On a target-specific basis, include alternates to be selected by IFUNC. if HAVE_IFUNC +if ARCH_AARCH64_LINUX_LSE +IFUNC_OPTIONS = -mcpu=thunderx2t99 +libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS))) +endif if ARCH_ARM_LINUX IFUNC_OPTIONS = -march=armv7-a -DHAVE_KERNEL64 libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS))) diff --git a/libatomic/config/linux/aarch64/host-config.h b/libatomic/config/linux/aarch64/host-config.h index e69de29..2e5e97a 100644 --- a/libatomic/config/linux/aarch64/host-config.h +++ b/libatomic/config/linux/aarch64/host-config.h @@ -0,0 +1,39 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + + This file is part of the GNU Atomic Library (libatomic). + + Libatomic is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + Libatomic is distributed in the hope that it will be useful, but WITHOUT ANY + WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + FOR A PARTICULAR PURPOSE. See the GNU General Public License for + more details. + + Under Section 7 of GPL version 3, you are granted additional + permissions described in the GCC Runtime Library Exception, version + 3.1, as published by the Free Software Foundation. + + You should have received a copy of the GNU General Public License and + a copy of the GCC Runtime Library Exception along with this program; + see the files COPYING3 and COPYING.RUNTIME respectively. If not, see + <http://www.gnu.org/licenses/>. */ + +#if HAVE_IFUNC +#include <stdlib.h> +#ifdef HAVE_SYS_AUXV_H +# include <sys/auxv.h> +#endif + +# ifdef HWCAP_ATOMICS +# define IFUNC_COND_1 (hwcap & HWCAP_ATOMICS) +# else +# define IFUNC_COND_1 (false) +# endif +# define IFUNC_NCOND(N) (1) + +#endif /* HAVE_IFUNC */ + +#include_next <host-config.h> diff --git a/libatomic/configure.ac b/libatomic/configure.ac index 023f172..4e06ffe 100644 --- a/libatomic/configure.ac +++ b/libatomic/configure.ac @@ -163,6 +163,10 @@ if test -n "$UNSUPPORTED"; then AC_MSG_ERROR([Configuration ${target} is unsupported.]) fi +# Write out the ifunc resolver arg type. +AC_DEFINE_UNQUOTED(HWCAP_TYPE, $HWCAP_TYPE, + [Define type of ifunc resolver function argument.]) + # Disable fallbacks to __sync routines from libgcc. Otherwise we'll # make silly decisions about what the cpu can do. CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS" @@ -171,7 +175,8 @@ CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS" AC_STDC_HEADERS ACX_HEADER_STRING GCC_HEADER_STDINT(gstdint.h) -AC_CHECK_HEADERS([fenv.h]) +AC_CHECK_HEADERS([fenv.h sys/auxv.h]) +AC_CHECK_FUNCS(getauxval) # Check for common type sizes LIBAT_FORALL_MODES([LIBAT_HAVE_INT_MODE]) @@ -247,6 +252,8 @@ AC_SUBST(LIBS) AC_SUBST(SIZES) AM_CONDITIONAL(HAVE_IFUNC, test x$libat_cv_have_ifunc = xyes) +AM_CONDITIONAL(ARCH_AARCH64_LINUX_LSE, + [expr "$config_path" : ".* linux/aarch64 .*" > /dev/null]) AM_CONDITIONAL(ARCH_ARM_LINUX, [expr "$config_path" : ".* linux/arm .*" > /dev/null]) AM_CONDITIONAL(ARCH_I386, diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt index b8af3ab..0bb5c66 100644 --- a/libatomic/configure.tgt +++ b/libatomic/configure.tgt @@ -40,6 +40,14 @@ case "${target_cpu}" in riscv*) ARCH=riscv ;; sh*) ARCH=sh ;; + aarch64*) + ARCH=aarch64 + case "${target}" in + aarch64*-*-linux*) + try_ifunc=yes + ;; + esac + ;; arm*) ARCH=arm case "${target}" in @@ -109,6 +117,11 @@ fi # Other system configury case "${target}" in + aarch64*-*-linux*) + # OS support for atomic primitives. + config_path="${config_path} linux/aarch64 posix" + ;; + arm*-*-linux*) # OS support for atomic primitives. config_path="${config_path} linux/arm posix" @@ -153,3 +166,14 @@ case "${target}" in UNSUPPORTED=1 ;; esac + +# glibc will pass hwcap to ifunc resolver functions as an argument. +# The type may be different on different architectures. +case "${target}" in + aarch64*-*-*) + HWCAP_TYPE=uint64_t + ;; + *) + HWCAP_TYPE="unsigned long int" + ;; +esac diff --git a/libatomic/libatomic_i.h b/libatomic/libatomic_i.h index 4eb372a..e7f2050 100644 --- a/libatomic/libatomic_i.h +++ b/libatomic/libatomic_i.h @@ -240,7 +240,7 @@ bool libat_is_lock_free (size_t, void *) MAN(is_lock_free); # if IFUNC_NCOND(N) == 1 # define GEN_SELECTOR(X) \ extern typeof(C2(libat_,X)) C3(libat_,X,_i1) HIDDEN; \ - static void * C2(select_,X) (void) \ + static void * C2(select_,X) (HWCAP_TYPE hwcap) \ { \ if (IFUNC_COND_1) \ return C3(libat_,X,_i1); \ @@ -250,7 +250,7 @@ bool libat_is_lock_free (size_t, void *) MAN(is_lock_free); # define GEN_SELECTOR(X) \ extern typeof(C2(libat_,X)) C3(libat_,X,_i1) HIDDEN; \ extern typeof(C2(libat_,X)) C3(libat_,X,_i2) HIDDEN; \ - static void * C2(select_,X) (void) \ + static void * C2(select_,X) (HWCAP_TYPE hwcap) \ { \ if (IFUNC_COND_1) \ return C3(libat_,X,_i1); \ @@ -263,7 +263,7 @@ bool libat_is_lock_free (size_t, void *) MAN(is_lock_free); extern typeof(C2(libat_,X)) C3(libat_,X,_i1) HIDDEN; \ extern typeof(C2(libat_,X)) C3(libat_,X,_i2) HIDDEN; \ extern typeof(C2(libat_,X)) C3(libat_,X,_i3) HIDDEN; \ - static void * C2(select_,X) (void) \ + static void * C2(select_,X) (HWCAP_TYPE hwcap) \ { \ if (IFUNC_COND_1) \ return C3(libat_,X,_i1); \ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-08-31 18:55 ` Steve Ellcey @ 2017-09-27 20:35 ` Steve Ellcey 2017-09-28 11:31 ` Szabolcs Nagy 1 sibling, 0 replies; 22+ messages in thread From: Steve Ellcey @ 2017-09-27 20:35 UTC (permalink / raw) To: Szabolcs Nagy, gcc-patches Cc: nd, richard.earnshaw, james.greenhalgh, Marcus Shawcroft Ping. Steve Ellcey sellcey@cavium.com On Thu, 2017-08-31 at 10:24 -0700, Steve Ellcey wrote: > On Tue, 2017-08-29 at 12:25 +0100, Szabolcs Nagy wrote: > > > >  > > in glibc the hwcap is not used, because it has accesses to > > cached dispatch info, but in libatomic using the hwcap > > argument is the right way. > Here is an updated version of the patch to allow aarch64 to use > ifuncs > in libatomic. > > The main difference from the last patch is that the library does not > access the hwcap value directly but accesses it through the ifunc > resolver argument.  That means that we no longer need the > init_cpu_revision static constructor to set a flag that the resolver > checks, instead the resolver just does a comparision of its incoming > argument with HWCAP_ATOMICS. > > This did mean I had to change the prototype for the resolver > functions > in libatomic_i.h to have an argument, which is the way glibc calls > them.  One complication of this is that the type of the argument can > differ between platforms and ABIs so I added code to configure.tgt to > set the type.  I used uint64_t for aarch64 and 'long unsigned int' > for everything else.  That is not correct for all platforms but at > this point no other platforms access the argument so it should not > matter.  If and when platforms do need to access it they can change > the type if necessary. > > Steve Ellcey > sellcey@cavium.com > > > 2017-08-31  Steve Ellcey  <sellcey@cavium.com> > > * Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and > libatomic_la_LIBADD. > * config/linux/aarch64/host-config.h: New file. > * configure.ac (HWCAP_TYPE): Define. > (AC_CHECK_HEADERS): Check for sys/auxv.h. > (AC_CHECK_FUNCS): Check for getauxval. > (ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds. > * configure.tgt (aarch64): Set AARCH and try_ifunc. > (aarch64*-*-linux*) Update config_path. > (aarch64*-*-linux*) Set HWCAP_TYPE. > * libatomic_i.h (GEN_SELECTOR): Add "HWCAP_TYPE hwcap" > argument. > * Makefile.in: Regenerate. > * auto-config.h.in: Regenerate. > * configure: Regenerate. > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-08-31 18:55 ` Steve Ellcey 2017-09-27 20:35 ` Steve Ellcey @ 2017-09-28 11:31 ` Szabolcs Nagy 2017-09-29 20:29 ` Steve Ellcey 1 sibling, 1 reply; 22+ messages in thread From: Szabolcs Nagy @ 2017-09-28 11:31 UTC (permalink / raw) To: sellcey, gcc-patches; +Cc: nd On 31/08/17 18:24, Steve Ellcey wrote: > On Tue, 2017-08-29 at 12:25 +0100, Szabolcs Nagy wrote: >> > >> > in glibc the hwcap is not used, because it has accesses to >> > cached dispatch info, but in libatomic using the hwcap >> > argument is the right way. > Here is an updated version of the patch to allow aarch64 to use ifuncs > in libatomic. > > The main difference from the last patch is that the library does not > access the hwcap value directly but accesses it through the ifunc > resolver argument. That means that we no longer need the > init_cpu_revision static constructor to set a flag that the resolver > checks, instead the resolver just does a comparision of its incoming > argument with HWCAP_ATOMICS. > i think this approach is fine. > This did mean I had to change the prototype for the resolver functions > in libatomic_i.h to have an argument, which is the way glibc calls > them. One complication of this is that the type of the argument can > differ between platforms and ABIs so I added code to configure.tgt to > set the type. I used uint64_t for aarch64 and 'long unsigned int' > for everything else. That is not correct for all platforms but at > this point no other platforms access the argument so it should not > matter. If and when platforms do need to access it they can change > the type if necessary. > i think this should be improved, see below. > Steve Ellcey > sellcey@cavium.com > > > 2017-08-31 Steve Ellcey <sellcey@cavium.com> > > * Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and > libatomic_la_LIBADD. > * config/linux/aarch64/host-config.h: New file. > * configure.ac (HWCAP_TYPE): Define. > (AC_CHECK_HEADERS): Check for sys/auxv.h. > (AC_CHECK_FUNCS): Check for getauxval. > (ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds. > * configure.tgt (aarch64): Set AARCH and try_ifunc. > (aarch64*-*-linux*) Update config_path. > (aarch64*-*-linux*) Set HWCAP_TYPE. > * libatomic_i.h (GEN_SELECTOR): Add "HWCAP_TYPE hwcap" argument. > * Makefile.in: Regenerate. > * auto-config.h.in: Regenerate. > * configure: Regenerate. > > > libatomic.patch > > > diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am > index d731406..a35df1e 100644 > --- a/libatomic/Makefile.am > +++ b/libatomic/Makefile.am > @@ -122,6 +122,10 @@ libatomic_la_LIBADD = $(foreach s,$(SIZES),$(addsuffix _$(s)_.lo,$(SIZEOBJS))) > > ## On a target-specific basis, include alternates to be selected by IFUNC. > if HAVE_IFUNC > +if ARCH_AARCH64_LINUX_LSE > +IFUNC_OPTIONS = -mcpu=thunderx2t99 i'd expect -march=armv8.1-a instead of a particular cpu here. (and it think this assumes a not very old binutils gas) > +libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS))) > +endif > if ARCH_ARM_LINUX > IFUNC_OPTIONS = -march=armv7-a -DHAVE_KERNEL64 > libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS))) > diff --git a/libatomic/configure.ac b/libatomic/configure.ac > index 023f172..4e06ffe 100644 > --- a/libatomic/configure.ac > +++ b/libatomic/configure.ac > @@ -163,6 +163,10 @@ if test -n "$UNSUPPORTED"; then > AC_MSG_ERROR([Configuration ${target} is unsupported.]) > fi > > +# Write out the ifunc resolver arg type. > +AC_DEFINE_UNQUOTED(HWCAP_TYPE, $HWCAP_TYPE, > + [Define type of ifunc resolver function argument.]) > + > # Disable fallbacks to __sync routines from libgcc. Otherwise we'll > # make silly decisions about what the cpu can do. > CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS" > @@ -171,7 +175,8 @@ CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS" > AC_STDC_HEADERS > ACX_HEADER_STRING > GCC_HEADER_STDINT(gstdint.h) > -AC_CHECK_HEADERS([fenv.h]) > +AC_CHECK_HEADERS([fenv.h sys/auxv.h]) > +AC_CHECK_FUNCS(getauxval) > getauxval is no longer needed. > # Check for common type sizes > LIBAT_FORALL_MODES([LIBAT_HAVE_INT_MODE]) > @@ -247,6 +252,8 @@ AC_SUBST(LIBS) > AC_SUBST(SIZES) > > AM_CONDITIONAL(HAVE_IFUNC, test x$libat_cv_have_ifunc = xyes) > +AM_CONDITIONAL(ARCH_AARCH64_LINUX_LSE, > + [expr "$config_path" : ".* linux/aarch64 .*" > /dev/null]) linux/aarch64 seems to be set for all aarch64*-*-linux* targets below. so why call it _LSE? > AM_CONDITIONAL(ARCH_ARM_LINUX, > [expr "$config_path" : ".* linux/arm .*" > /dev/null]) > AM_CONDITIONAL(ARCH_I386, > diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt > index b8af3ab..0bb5c66 100644 > --- a/libatomic/configure.tgt > +++ b/libatomic/configure.tgt > @@ -40,6 +40,14 @@ case "${target_cpu}" in > riscv*) ARCH=riscv ;; > sh*) ARCH=sh ;; > > + aarch64*) > + ARCH=aarch64 > + case "${target}" in > + aarch64*-*-linux*) > + try_ifunc=yes > + ;; > + esac > + ;; > arm*) > ARCH=arm > case "${target}" in > @@ -109,6 +117,11 @@ fi > > # Other system configury > case "${target}" in > + aarch64*-*-linux*) > + # OS support for atomic primitives. > + config_path="${config_path} linux/aarch64 posix" > + ;; > + > arm*-*-linux*) > # OS support for atomic primitives. > config_path="${config_path} linux/arm posix" > @@ -153,3 +166,14 @@ case "${target}" in > UNSUPPORTED=1 > ;; > esac > + > +# glibc will pass hwcap to ifunc resolver functions as an argument. > +# The type may be different on different architectures. > +case "${target}" in > + aarch64*-*-*) > + HWCAP_TYPE=uint64_t > + ;; > + *) > + HWCAP_TYPE="unsigned long int" > + ;; the resolver type on x86_64 is void in principle (and on other targets it may take multiple args) so maybe use IFUNC_RESOLVER_ARGS='uint64_t hwcap' and IFUNC_RESOLVER_ARGS='void' ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-09-28 11:31 ` Szabolcs Nagy @ 2017-09-29 20:29 ` Steve Ellcey 2017-10-02 14:38 ` Szabolcs Nagy 2017-12-07 9:56 ` James Greenhalgh 0 siblings, 2 replies; 22+ messages in thread From: Steve Ellcey @ 2017-09-29 20:29 UTC (permalink / raw) To: Szabolcs Nagy, gcc-patches; +Cc: nd [-- Attachment #1: Type: text/plain, Size: 1028 bytes --] On Thu, 2017-09-28 at 12:31 +0100, Szabolcs Nagy wrote: > > i think this should be improved, see below. Those were all good suggestions, here is a new patch that incorporates the changes.  I fixed the IFUNC_OPTIONS argument, renamed ARCH_AARCH64_LINUX_LSE, got rid of the auxv references, and changed HWCAP_TYPE to IFUNC_RESOLVER_ARGS. Here is the new patch, tested with no regressions. Steve Ellcey sellcey@cavium.com 2017-09-29  Steve Ellcey  <sellcey@cavium.com> * Makefile.am (ARCH_AARCH64_LINUX): Add IFUNC_OPTIONS and libatomic_la_LIBADD. * config/linux/aarch64/host-config.h: New file. * configure.ac (IFUNC_RESOLVER_ARGS): Define. (ARCH_AARCH64_LINUX): New conditional for IFUNC builds. * configure.tgt (aarch64): Set ARCH and try_ifunc. (aarch64*-*-linux*) Update config_path. (aarch64*-*-linux*) Set IFUNC_RESOLVER_ARGS. * libatomic_i.h (GEN_SELECTOR): Add IFUNC_RESOLVER_ARGS argument. * Makefile.in: Regenerate. * auto-config.h.in: Regenerate. * configure: Regenerate. [-- Attachment #2: libatomic.patch --] [-- Type: text/x-patch, Size: 5518 bytes --] diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am index d731406..92d19c6 100644 --- a/libatomic/Makefile.am +++ b/libatomic/Makefile.am @@ -122,6 +122,10 @@ libatomic_la_LIBADD = $(foreach s,$(SIZES),$(addsuffix _$(s)_.lo,$(SIZEOBJS))) ## On a target-specific basis, include alternates to be selected by IFUNC. if HAVE_IFUNC +if ARCH_AARCH64_LINUX +IFUNC_OPTIONS = -march=armv8.1-a +libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS))) +endif if ARCH_ARM_LINUX IFUNC_OPTIONS = -march=armv7-a -DHAVE_KERNEL64 libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS))) diff --git a/libatomic/config/linux/aarch64/host-config.h b/libatomic/config/linux/aarch64/host-config.h index e69de29..08810a9 100644 --- a/libatomic/config/linux/aarch64/host-config.h +++ b/libatomic/config/linux/aarch64/host-config.h @@ -0,0 +1,36 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + + This file is part of the GNU Atomic Library (libatomic). + + Libatomic is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + Libatomic is distributed in the hope that it will be useful, but WITHOUT ANY + WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + FOR A PARTICULAR PURPOSE. See the GNU General Public License for + more details. + + Under Section 7 of GPL version 3, you are granted additional + permissions described in the GCC Runtime Library Exception, version + 3.1, as published by the Free Software Foundation. + + You should have received a copy of the GNU General Public License and + a copy of the GCC Runtime Library Exception along with this program; + see the files COPYING3 and COPYING.RUNTIME respectively. If not, see + <http://www.gnu.org/licenses/>. */ + +#if HAVE_IFUNC +#include <stdlib.h> + +# ifdef HWCAP_ATOMICS +# define IFUNC_COND_1 (hwcap & HWCAP_ATOMICS) +# else +# define IFUNC_COND_1 (false) +# endif +# define IFUNC_NCOND(N) (1) + +#endif /* HAVE_IFUNC */ + +#include_next <host-config.h> diff --git a/libatomic/configure.ac b/libatomic/configure.ac index 023f172..b717d3d 100644 --- a/libatomic/configure.ac +++ b/libatomic/configure.ac @@ -163,6 +163,10 @@ if test -n "$UNSUPPORTED"; then AC_MSG_ERROR([Configuration ${target} is unsupported.]) fi +# Write out the ifunc resolver arg type. +AC_DEFINE_UNQUOTED(IFUNC_RESOLVER_ARGS, $IFUNC_RESOLVER_ARGS, + [Define ifunc resolver function argument.]) + # Disable fallbacks to __sync routines from libgcc. Otherwise we'll # make silly decisions about what the cpu can do. CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS" @@ -247,6 +251,8 @@ AC_SUBST(LIBS) AC_SUBST(SIZES) AM_CONDITIONAL(HAVE_IFUNC, test x$libat_cv_have_ifunc = xyes) +AM_CONDITIONAL(ARCH_AARCH64_LINUX, + [expr "$config_path" : ".* linux/aarch64 .*" > /dev/null]) AM_CONDITIONAL(ARCH_ARM_LINUX, [expr "$config_path" : ".* linux/arm .*" > /dev/null]) AM_CONDITIONAL(ARCH_I386, diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt index b8af3ab..388ae95 100644 --- a/libatomic/configure.tgt +++ b/libatomic/configure.tgt @@ -40,6 +40,14 @@ case "${target_cpu}" in riscv*) ARCH=riscv ;; sh*) ARCH=sh ;; + aarch64*) + ARCH=aarch64 + case "${target}" in + aarch64*-*-linux*) + try_ifunc=yes + ;; + esac + ;; arm*) ARCH=arm case "${target}" in @@ -109,6 +117,11 @@ fi # Other system configury case "${target}" in + aarch64*-*-linux*) + # OS support for atomic primitives. + config_path="${config_path} linux/aarch64 posix" + ;; + arm*-*-linux*) # OS support for atomic primitives. config_path="${config_path} linux/arm posix" @@ -153,3 +166,14 @@ case "${target}" in UNSUPPORTED=1 ;; esac + +# glibc will pass hwcap to ifunc resolver functions as an argument. +# The type may be different on different architectures. +case "${target}" in + aarch64*-*-*) + IFUNC_RESOLVER_ARGS="uint64_t hwcap" + ;; + *) + IFUNC_RESOLVER_ARGS="void" + ;; +esac diff --git a/libatomic/libatomic_i.h b/libatomic/libatomic_i.h index 4eb372a..b25f2f3 100644 --- a/libatomic/libatomic_i.h +++ b/libatomic/libatomic_i.h @@ -240,7 +240,7 @@ bool libat_is_lock_free (size_t, void *) MAN(is_lock_free); # if IFUNC_NCOND(N) == 1 # define GEN_SELECTOR(X) \ extern typeof(C2(libat_,X)) C3(libat_,X,_i1) HIDDEN; \ - static void * C2(select_,X) (void) \ + static void * C2(select_,X) (IFUNC_RESOLVER_ARGS) \ { \ if (IFUNC_COND_1) \ return C3(libat_,X,_i1); \ @@ -250,7 +250,7 @@ bool libat_is_lock_free (size_t, void *) MAN(is_lock_free); # define GEN_SELECTOR(X) \ extern typeof(C2(libat_,X)) C3(libat_,X,_i1) HIDDEN; \ extern typeof(C2(libat_,X)) C3(libat_,X,_i2) HIDDEN; \ - static void * C2(select_,X) (void) \ + static void * C2(select_,X) (IFUNC_RESOLVER_ARGS) \ { \ if (IFUNC_COND_1) \ return C3(libat_,X,_i1); \ @@ -263,7 +263,7 @@ bool libat_is_lock_free (size_t, void *) MAN(is_lock_free); extern typeof(C2(libat_,X)) C3(libat_,X,_i1) HIDDEN; \ extern typeof(C2(libat_,X)) C3(libat_,X,_i2) HIDDEN; \ extern typeof(C2(libat_,X)) C3(libat_,X,_i3) HIDDEN; \ - static void * C2(select_,X) (void) \ + static void * C2(select_,X) (IFUNC_RESOLVER_ARGS) \ { \ if (IFUNC_COND_1) \ return C3(libat_,X,_i1); \ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-09-29 20:29 ` Steve Ellcey @ 2017-10-02 14:38 ` Szabolcs Nagy 2017-10-03 18:57 ` Steve Ellcey 2017-12-07 9:56 ` James Greenhalgh 1 sibling, 1 reply; 22+ messages in thread From: Szabolcs Nagy @ 2017-10-02 14:38 UTC (permalink / raw) To: sellcey, gcc-patches; +Cc: nd On 29/09/17 21:29, Steve Ellcey wrote: > On Thu, 2017-09-28 at 12:31 +0100, Szabolcs Nagy wrote: >> >> i think this should be improved, see below. > > Those were all good suggestions, here is a new patch that incorporates > the changes. I fixed the IFUNC_OPTIONS argument, > renamed ARCH_AARCH64_LINUX_LSE, got rid of the auxv references, and > changed HWCAP_TYPE to IFUNC_RESOLVER_ARGS. > > Here is the new patch, tested with no regressions. > looks good to me, but i cannot approve. (this will make libatomic depend on ifuncs on aarch64*-linux-gnu.) > Steve Ellcey > sellcey@cavium.com > > > 2017-09-29 Steve Ellcey <sellcey@cavium.com> > > * Makefile.am (ARCH_AARCH64_LINUX): Add IFUNC_OPTIONS and > libatomic_la_LIBADD. > * config/linux/aarch64/host-config.h: New file. > * configure.ac (IFUNC_RESOLVER_ARGS): Define. > (ARCH_AARCH64_LINUX): New conditional for IFUNC builds. > * configure.tgt (aarch64): Set ARCH and try_ifunc. > (aarch64*-*-linux*) Update config_path. > (aarch64*-*-linux*) Set IFUNC_RESOLVER_ARGS. > * libatomic_i.h (GEN_SELECTOR): Add IFUNC_RESOLVER_ARGS argument. > * Makefile.in: Regenerate. > * auto-config.h.in: Regenerate. > * configure: Regenerate. > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-10-02 14:38 ` Szabolcs Nagy @ 2017-10-03 18:57 ` Steve Ellcey 2017-10-24 18:17 ` Steve Ellcey 0 siblings, 1 reply; 22+ messages in thread From: Steve Ellcey @ 2017-10-03 18:57 UTC (permalink / raw) To: Szabolcs Nagy, gcc-patches; +Cc: nd [-- Attachment #1: Type: text/plain, Size: 1731 bytes --] On Mon, 2017-10-02 at 15:38 +0100, Szabolcs Nagy wrote: > > looks good to me, but i cannot approve. > > (this will make libatomic depend on ifuncs on aarch64*-linux-gnu.) If you build GCC with default options, ifunc is enabled on aarch64 and used by libatomic but if you configure with '--disable-gnu-indirect- function' then GCC will not recognize the 'resolver' attribute and libatomic will build the 'old' way and not use IFUNC's. It seems like using '--disable-version-specific-runtime-libs' should allow GCC to recognize the 'resolver' attribute for IFUNCs but to not use them when building libatomic but when I tried that I got ifuncs in libatomic anyway.  I think that is a bug and I will look into it some more. The recent alias checking improvements made to GCC and which caused some glibc build problems also caused the ifunc check in libatomic to fail.  That problem has been fixed but it involved a change to libatomic/ibatomic_i.h that conflicted with this patch so I am including an updated version that will apply cleanly to the top of tree.  Only libatomic_i.h changed. Steve Ellcey sellcey@cavium.com 2017-10-03  Steve Ellcey  <sellcey@cavium.com> * Makefile.am (ARCH_AARCH64_LINUX): Add IFUNC_OPTIONS and libatomic_la_LIBADD. * config/linux/aarch64/host-config.h: New file. * configure.ac (IFUNC_RESOLVER_ARGS): Define. (ARCH_AARCH64_LINUX): New conditional for IFUNC builds. * configure.tgt (aarch64): Set ARCH and try_ifunc. (aarch64*-*-linux*) Update config_path. (aarch64*-*-linux*) Set IFUNC_RESOLVER_ARGS. * libatomic_i.h (GEN_SELECTOR): Add IFUNC_RESOLVER_ARGS argument. * Makefile.in: Regenerate. * auto-config.h.in: Regenerate. * configure: Regenerate. [-- Attachment #2: libatomic.patch --] [-- Type: text/x-patch, Size: 5608 bytes --] diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am index d731406..92d19c6 100644 --- a/libatomic/Makefile.am +++ b/libatomic/Makefile.am @@ -122,6 +122,10 @@ libatomic_la_LIBADD = $(foreach s,$(SIZES),$(addsuffix _$(s)_.lo,$(SIZEOBJS))) ## On a target-specific basis, include alternates to be selected by IFUNC. if HAVE_IFUNC +if ARCH_AARCH64_LINUX +IFUNC_OPTIONS = -march=armv8.1-a +libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS))) +endif if ARCH_ARM_LINUX IFUNC_OPTIONS = -march=armv7-a -DHAVE_KERNEL64 libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS))) diff --git a/libatomic/config/linux/aarch64/host-config.h b/libatomic/config/linux/aarch64/host-config.h index e69de29..08810a9 100644 --- a/libatomic/config/linux/aarch64/host-config.h +++ b/libatomic/config/linux/aarch64/host-config.h @@ -0,0 +1,36 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + + This file is part of the GNU Atomic Library (libatomic). + + Libatomic is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + Libatomic is distributed in the hope that it will be useful, but WITHOUT ANY + WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + FOR A PARTICULAR PURPOSE. See the GNU General Public License for + more details. + + Under Section 7 of GPL version 3, you are granted additional + permissions described in the GCC Runtime Library Exception, version + 3.1, as published by the Free Software Foundation. + + You should have received a copy of the GNU General Public License and + a copy of the GCC Runtime Library Exception along with this program; + see the files COPYING3 and COPYING.RUNTIME respectively. If not, see + <http://www.gnu.org/licenses/>. */ + +#if HAVE_IFUNC +#include <stdlib.h> + +# ifdef HWCAP_ATOMICS +# define IFUNC_COND_1 (hwcap & HWCAP_ATOMICS) +# else +# define IFUNC_COND_1 (false) +# endif +# define IFUNC_NCOND(N) (1) + +#endif /* HAVE_IFUNC */ + +#include_next <host-config.h> diff --git a/libatomic/configure.ac b/libatomic/configure.ac index 023f172..b717d3d 100644 --- a/libatomic/configure.ac +++ b/libatomic/configure.ac @@ -163,6 +163,10 @@ if test -n "$UNSUPPORTED"; then AC_MSG_ERROR([Configuration ${target} is unsupported.]) fi +# Write out the ifunc resolver arg type. +AC_DEFINE_UNQUOTED(IFUNC_RESOLVER_ARGS, $IFUNC_RESOLVER_ARGS, + [Define ifunc resolver function argument.]) + # Disable fallbacks to __sync routines from libgcc. Otherwise we'll # make silly decisions about what the cpu can do. CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS" @@ -247,6 +251,8 @@ AC_SUBST(LIBS) AC_SUBST(SIZES) AM_CONDITIONAL(HAVE_IFUNC, test x$libat_cv_have_ifunc = xyes) +AM_CONDITIONAL(ARCH_AARCH64_LINUX, + [expr "$config_path" : ".* linux/aarch64 .*" > /dev/null]) AM_CONDITIONAL(ARCH_ARM_LINUX, [expr "$config_path" : ".* linux/arm .*" > /dev/null]) AM_CONDITIONAL(ARCH_I386, diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt index b8af3ab..388ae95 100644 --- a/libatomic/configure.tgt +++ b/libatomic/configure.tgt @@ -40,6 +40,14 @@ case "${target_cpu}" in riscv*) ARCH=riscv ;; sh*) ARCH=sh ;; + aarch64*) + ARCH=aarch64 + case "${target}" in + aarch64*-*-linux*) + try_ifunc=yes + ;; + esac + ;; arm*) ARCH=arm case "${target}" in @@ -109,6 +117,11 @@ fi # Other system configury case "${target}" in + aarch64*-*-linux*) + # OS support for atomic primitives. + config_path="${config_path} linux/aarch64 posix" + ;; + arm*-*-linux*) # OS support for atomic primitives. config_path="${config_path} linux/arm posix" @@ -153,3 +166,14 @@ case "${target}" in UNSUPPORTED=1 ;; esac + +# glibc will pass hwcap to ifunc resolver functions as an argument. +# The type may be different on different architectures. +case "${target}" in + aarch64*-*-*) + IFUNC_RESOLVER_ARGS="uint64_t hwcap" + ;; + *) + IFUNC_RESOLVER_ARGS="void" + ;; +esac diff --git a/libatomic/libatomic_i.h b/libatomic/libatomic_i.h index 2dad4a8..2ecc27a 100644 --- a/libatomic/libatomic_i.h +++ b/libatomic/libatomic_i.h @@ -240,7 +240,7 @@ bool libat_is_lock_free (size_t, void *) MAN(is_lock_free); # if IFUNC_NCOND(N) == 1 # define GEN_SELECTOR(X) \ extern typeof(C2(libat_,X)) C3(libat_,X,_i1) HIDDEN; \ - static typeof(C2(libat_,X)) * C2(select_,X) (void) \ + static typeof(C2(libat_,X)) * C2(select_,X) (IFUNC_RESOLVER_ARGS) \ { \ if (IFUNC_COND_1) \ return C3(libat_,X,_i1); \ @@ -250,7 +250,7 @@ bool libat_is_lock_free (size_t, void *) MAN(is_lock_free); # define GEN_SELECTOR(X) \ extern typeof(C2(libat_,X)) C3(libat_,X,_i1) HIDDEN; \ extern typeof(C2(libat_,X)) C3(libat_,X,_i2) HIDDEN; \ - static typeof(C2(libat_,X)) * C2(select_,X) (void) \ + static typeof(C2(libat_,X)) * C2(select_,X) (IFUNC_RESOLVER_ARGS) \ { \ if (IFUNC_COND_1) \ return C3(libat_,X,_i1); \ @@ -263,7 +263,7 @@ bool libat_is_lock_free (size_t, void *) MAN(is_lock_free); extern typeof(C2(libat_,X)) C3(libat_,X,_i1) HIDDEN; \ extern typeof(C2(libat_,X)) C3(libat_,X,_i2) HIDDEN; \ extern typeof(C2(libat_,X)) C3(libat_,X,_i3) HIDDEN; \ - static typeof(C2(libat_,X)) * C2(select_,X) (void) \ + static typeof(C2(libat_,X)) * C2(select_,X) (IFUNC_RESOLVER_ARGS) \ { \ if (IFUNC_COND_1) \ return C3(libat_,X,_i1); \ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-10-03 18:57 ` Steve Ellcey @ 2017-10-24 18:17 ` Steve Ellcey 2017-11-20 18:27 ` Steve Ellcey 0 siblings, 1 reply; 22+ messages in thread From: Steve Ellcey @ 2017-10-24 18:17 UTC (permalink / raw) To: Szabolcs Nagy, gcc-patches; +Cc: nd Ping. Steve Ellcey sellcey@cavium.com On Tue, 2017-10-03 at 11:57 -0700, Steve Ellcey wrote: > On Mon, 2017-10-02 at 15:38 +0100, Szabolcs Nagy wrote: > > > >  > > looks good to me, but i cannot approve. > > > > (this will make libatomic depend on ifuncs on aarch64*-linux-gnu.) > If you build GCC with default options, ifunc is enabled on aarch64 > and > used by libatomic but if you configure with '--disable-gnu-indirect- > function' then GCC will not recognize the 'resolver' attribute and > libatomic will build the 'old' way and not use IFUNC's. > > It seems like using '--disable-version-specific-runtime-libs' should > allow GCC to recognize the 'resolver' attribute for IFUNCs but to not > use them when building libatomic but when I tried that I got ifuncs > in > libatomic anyway.  I think that is a bug and I will look into it some > more. > > The recent alias checking improvements made to GCC and which caused > some glibc build problems also caused the ifunc check in libatomic to > fail.  That problem has been fixed but it involved a change to > libatomic/ibatomic_i.h that conflicted with this patch so I am > including an updated version that will apply cleanly to the top of > tree.  Only libatomic_i.h changed. > > Steve Ellcey > sellcey@cavium.com > > 2017-10-03  Steve Ellcey  <sellcey@cavium.com> > > * Makefile.am (ARCH_AARCH64_LINUX): Add IFUNC_OPTIONS and > libatomic_la_LIBADD. > * config/linux/aarch64/host-config.h: New file. > * configure.ac (IFUNC_RESOLVER_ARGS): Define. > (ARCH_AARCH64_LINUX): New conditional for IFUNC builds. > * configure.tgt (aarch64): Set ARCH and try_ifunc. > (aarch64*-*-linux*) Update config_path. > (aarch64*-*-linux*) Set IFUNC_RESOLVER_ARGS. > * libatomic_i.h (GEN_SELECTOR): Add IFUNC_RESOLVER_ARGS > argument. > * Makefile.in: Regenerate. > * auto-config.h.in: Regenerate. > * configure: Regenerate. > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-10-24 18:17 ` Steve Ellcey @ 2017-11-20 18:27 ` Steve Ellcey 2017-11-20 18:29 ` James Greenhalgh 0 siblings, 1 reply; 22+ messages in thread From: Steve Ellcey @ 2017-11-20 18:27 UTC (permalink / raw) To: Szabolcs Nagy, gcc-patches Cc: nd, james.greenhalgh, richard.earnshaw, Marcus Shawcroft Re-ping with a CC to the Aarch64 maintainers. Steve Ellcey sellcey@cavium.com On Tue, 2017-10-24 at 11:11 -0700, Steve Ellcey wrote: > Ping. > > Steve Ellcey > sellcey@cavium.com > > On Tue, 2017-10-03 at 11:57 -0700, Steve Ellcey wrote: > > > > On Mon, 2017-10-02 at 15:38 +0100, Szabolcs Nagy wrote: > > > > > > > > >  > > > looks good to me, but i cannot approve. > > > > > > (this will make libatomic depend on ifuncs on aarch64*-linux-gnu.) > > If you build GCC with default options, ifunc is enabled on aarch64 > > and > > used by libatomic but if you configure with '--disable-gnu- > > indirect- > > function' then GCC will not recognize the 'resolver' attribute and > > libatomic will build the 'old' way and not use IFUNC's. > > > > It seems like using '--disable-version-specific-runtime-libs' should > > allow GCC to recognize the 'resolver' attribute for IFUNCs but to not > > use them when building libatomic but when I tried that I got ifuncs > > in > > libatomic anyway.  I think that is a bug and I will look into it some > > more. > > > > The recent alias checking improvements made to GCC and which caused > > some glibc build problems also caused the ifunc check in libatomic to > > fail.  That problem has been fixed but it involved a change to > > libatomic/ibatomic_i.h that conflicted with this patch so I am > > including an updated version that will apply cleanly to the top of > > tree.  Only libatomic_i.h changed. > > > > Steve Ellcey > > sellcey@cavium.com > > > > 2017-10-03  Steve Ellcey  <sellcey@cavium.com> > > > > * Makefile.am (ARCH_AARCH64_LINUX): Add IFUNC_OPTIONS and > > libatomic_la_LIBADD. > > * config/linux/aarch64/host-config.h: New file. > > * configure.ac (IFUNC_RESOLVER_ARGS): Define. > > (ARCH_AARCH64_LINUX): New conditional for IFUNC builds. > > * configure.tgt (aarch64): Set ARCH and try_ifunc. > > (aarch64*-*-linux*) Update config_path. > > (aarch64*-*-linux*) Set IFUNC_RESOLVER_ARGS. > > * libatomic_i.h (GEN_SELECTOR): Add IFUNC_RESOLVER_ARGS > > argument. > > * Makefile.in: Regenerate. > > * auto-config.h.in: Regenerate. > > * configure: Regenerate. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-11-20 18:27 ` Steve Ellcey @ 2017-11-20 18:29 ` James Greenhalgh 2017-11-20 19:50 ` Steve Ellcey 0 siblings, 1 reply; 22+ messages in thread From: James Greenhalgh @ 2017-11-20 18:29 UTC (permalink / raw) To: Steve Ellcey Cc: Szabolcs Nagy, gcc-patches, nd, Richard Earnshaw, Marcus Shawcroft On Mon, Nov 20, 2017 at 05:39:25PM +0000, Steve Ellcey wrote: > Re-ping with a CC to the Aarch64 maintainers. If I'm completely honest with myself, I don't know enough about this area to review the patch. Szabolcs' OK holds a lot of weight with me, but I'd like to understand more of the top-level overview of what this patch means. If you have the time, would you mind giving me a quick run-down of what design decisions went in to this patch, and why they are the right thing to do? Sorry to offload that, but it will be the most efficient route to a review. Otherwise, I'll try and make some time for this once I've made it through the Stack-smashing and SVE reviews. (Or another maintainer might beat me to it :-) ) Thanks, James > > > > looks good to me, but i cannot approve. > > > > > > > > (this will make libatomic depend on ifuncs on aarch64*-linux-gnu.) > > > If you build GCC with default options, ifunc is enabled on aarch64 > > > and > > > used by libatomic but if you configure with '--disable-gnu- > > > indirect- > > > function' then GCC will not recognize the 'resolver' attribute and > > > libatomic will build the 'old' way and not use IFUNC's. > > > > > > It seems like using '--disable-version-specific-runtime-libs' should > > > allow GCC to recognize the 'resolver' attribute for IFUNCs but to not > > > use them when building libatomic but when I tried that I got ifuncs > > > in > > > libatomic anyway.  I think that is a bug and I will look into it some > > > more. > > > > > > The recent alias checking improvements made to GCC and which caused > > > some glibc build problems also caused the ifunc check in libatomic to > > > fail.  That problem has been fixed but it involved a change to > > > libatomic/ibatomic_i.h that conflicted with this patch so I am > > > including an updated version that will apply cleanly to the top of > > > tree.  Only libatomic_i.h changed. > > > > > > Steve Ellcey > > > sellcey@cavium.com > > > > > > 2017-10-03  Steve Ellcey  <sellcey@cavium.com> > > > > > > * Makefile.am (ARCH_AARCH64_LINUX): Add IFUNC_OPTIONS and > > > libatomic_la_LIBADD. > > > * config/linux/aarch64/host-config.h: New file. > > > * configure.ac (IFUNC_RESOLVER_ARGS): Define. > > > (ARCH_AARCH64_LINUX): New conditional for IFUNC builds. > > > * configure.tgt (aarch64): Set ARCH and try_ifunc. > > > (aarch64*-*-linux*) Update config_path. > > > (aarch64*-*-linux*) Set IFUNC_RESOLVER_ARGS. > > > * libatomic_i.h (GEN_SELECTOR): Add IFUNC_RESOLVER_ARGS > > > argument. > > > * Makefile.in: Regenerate. > > > * auto-config.h.in: Regenerate. > > > * configure: Regenerate. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-11-20 18:29 ` James Greenhalgh @ 2017-11-20 19:50 ` Steve Ellcey 2017-11-21 17:36 ` James Greenhalgh 0 siblings, 1 reply; 22+ messages in thread From: Steve Ellcey @ 2017-11-20 19:50 UTC (permalink / raw) To: James Greenhalgh Cc: Szabolcs Nagy, gcc-patches, nd, Richard Earnshaw, Marcus Shawcroft On Mon, 2017-11-20 at 18:27 +0000, James Greenhalgh wrote: > > If you have the time, would you mind giving me a quick run-down of what > design decisions went in to this patch, and why they are the right thing > to do? Sorry to offload that, but it will be the most efficient route > to a review. The main design decision was to use the existing IFUNC infrastructure that is used on ARM32 to enable atomic instructions that were added with armv7-a, on i386 to enable instructions added with i586, and on x86_64 to enable instructions added with cx16. The basic idea for all these is to allow users who create programs that use the atomic_* functions to use new instructions on machines that support them while also working on older machines that do not support them and to not have to create two separate executables. Some atomic_* functions get inlined into programs, and those will either use or not use LSE instructions based on the compiler arguments used during compilations.  If you want your program to work on all machines you have to not compile for LSE intructions.  But other functions (or all functions if -fno-inline-atomics is used) will call the libatomic library.  Currently those functions do not use LSE instructions but with this patch we can use the IFUNC infrastructure to check for LSE support and use LSE in libatomic on machines where it is supported or not use it on machines where it is not supported. As an example of what this change does, __atomic_compare_exchange_8 will turn into a call to libat_compare_exchange_8_i1 on a machine that supports LSE: 0000000000000000 <libat_compare_exchange_8_i1>:    0: f9400023 ldr x3, [x1]    4: aa0303e4 mov x4, x3    8: c8e4fc02 casal x4, x2, [x0]    c: eb03009f cmp x4, x3  10: 1a9f17e0 cset w0, eq  14: 35000040 cbnz w0, 1c <libat_compare_exchange_8_i1+0x1c>  18: f9000024 str x4, [x1]  1c: d65f03c0 ret But call libat_compare_exchange_8 on a machine without LSE: 0000000000000000 <libat_compare_exchange_8>:    0: f9400023 ldr x3, [x1]    4: c85ffc04 ldaxr x4, [x0]    8: eb03009f cmp x4, x3    c: 54000061 b.ne 18 <libat_compare_exchange_8+0x18>  10: c805fc02 stlxr w5, x2, [x0]  14: 35ffff85 cbnz w5, 4 <libat_compare_exchange_8+0x4>  18: 1a9f17e0 cset w0, eq  1c: 34000040 cbz w0, 24 <libat_compare_exchange_8+0x24>  20: d65f03c0 ret  24: f9000024 str x4, [x1]  28: d65f03c0 ret  2c: d503201f nop Steve Ellcey sellcey@cavium.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-11-20 19:50 ` Steve Ellcey @ 2017-11-21 17:36 ` James Greenhalgh 2017-11-29 8:09 ` Steve Ellcey 0 siblings, 1 reply; 22+ messages in thread From: James Greenhalgh @ 2017-11-21 17:36 UTC (permalink / raw) To: Steve Ellcey Cc: Szabolcs Nagy, gcc-patches, nd, Richard Earnshaw, Marcus Shawcroft On Mon, Nov 20, 2017 at 07:22:15PM +0000, Steve Ellcey wrote: > On Mon, 2017-11-20 at 18:27 +0000, James Greenhalgh wrote: > > > > If you have the time, would you mind giving me a quick run-down of what > > design decisions went in to this patch, and why they are the right thing > > to do? Sorry to offload that, but it will be the most efficient route > > to a review. > > The main design decision was to use the existing IFUNC infrastructure > that is used on ARM32 to enable atomic instructions that were added > with armv7-a, on i386 to enable instructions added with i586, and on > x86_64 to enable instructions added with cx16. > > The basic idea for all these is to allow users who create programs that > use the atomic_* functions to use new instructions on machines that > support them while also working on older machines that do not support > them and to not have to create two separate executables. > > Some atomic_* functions get inlined into programs, and those will > either use or not use LSE instructions based on the compiler arguments > used during compilations.  If you want your program to work on all > machines you have to not compile for LSE intructions.  But other > functions (or all functions if -fno-inline-atomics is used) will call > the libatomic library.  Currently those functions do not use LSE > instructions but with this patch we can use the IFUNC infrastructure to > check for LSE support and use LSE in libatomic on machines where it is > supported or not use it on machines where it is not supported. > > As an example of what this change does, __atomic_compare_exchange_8 will > turn into a call to libat_compare_exchange_8_i1 on a machine that supports > LSE: > > 0000000000000000 <libat_compare_exchange_8_i1>: >    0: f9400023 ldr x3, [x1] >    4: aa0303e4 mov x4, x3 >    8: c8e4fc02 casal x4, x2, [x0] >    c: eb03009f cmp x4, x3 >  10: 1a9f17e0 cset w0, eq >  14: 35000040 cbnz w0, 1c <libat_compare_exchange_8_i1+0x1c> >  18: f9000024 str x4, [x1] >  1c: d65f03c0 ret > > But call libat_compare_exchange_8 on a machine without LSE: > > 0000000000000000 <libat_compare_exchange_8>: >    0: f9400023 ldr x3, [x1] >    4: c85ffc04 ldaxr x4, [x0] >    8: eb03009f cmp x4, x3 >    c: 54000061 b.ne 18 <libat_compare_exchange_8+0x18> >  10: c805fc02 stlxr w5, x2, [x0] >  14: 35ffff85 cbnz w5, 4 <libat_compare_exchange_8+0x4> >  18: 1a9f17e0 cset w0, eq >  1c: 34000040 cbz w0, 24 <libat_compare_exchange_8+0x24> >  20: d65f03c0 ret >  24: f9000024 str x4, [x1] >  28: d65f03c0 ret >  2c: d503201f nop Thanks for the detailed explanation. I understood this, and my opinion is that the AArch64 parts of this patch are OK (and I don't know who needs to Ack the small generic changes you require). Let's give Richard/Marcus 48 hours to object while we wait for an OK on the generic bits, and then OK for AArch64. Thanks, James Reviewed-By: James Greenhalgh <james.greenhalgh@arm.com> ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-11-21 17:36 ` James Greenhalgh @ 2017-11-29 8:09 ` Steve Ellcey 2017-12-05 0:51 ` Steve Ellcey 0 siblings, 1 reply; 22+ messages in thread From: Steve Ellcey @ 2017-11-29 8:09 UTC (permalink / raw) To: James Greenhalgh Cc: Szabolcs Nagy, gcc-patches, nd, Richard Earnshaw, Marcus Shawcroft On Tue, 2017-11-21 at 17:35 +0000, James Greenhalgh wrote: > > Thanks for the detailed explanation. I understood this, and my opinion is > that the AArch64 parts of this patch are OK (and I don't know who needs to > Ack the small generic changes you require). > > Let's give Richard/Marcus 48 hours to object while we wait for an OK on the > generic bits, and then OK for AArch64. > > Thanks, > James > > Reviewed-By: James Greenhalgh <james.greenhalgh@arm.com> James, I haven't seen anything from Richard or Marcus.  Do you think a review from a global maintainer is really needed?  There is no libatomic specific maintainer.  The only non-aarch64 specific change is using the macro IFUNC_RESOLVER_ARGS in place of the hardcoded 'void' argument for ifunc selector functions and for all non-aarch64 platforms the macro will be defined as 'void' so there is no real change for any other platform. Steve Ellcey sellcey@cavium.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-11-29 8:09 ` Steve Ellcey @ 2017-12-05 0:51 ` Steve Ellcey 0 siblings, 0 replies; 22+ messages in thread From: Steve Ellcey @ 2017-12-05 0:51 UTC (permalink / raw) To: James Greenhalgh Cc: Szabolcs Nagy, gcc-patches, nd, Richard Earnshaw, Marcus Shawcroft FYI: Since James approved the Aarch64 part and since I think the generic part can be considered trivial, I have gone ahead and checked this patch in. Steve Ellcey sellcey@cavium.com On Tue, 2017-11-28 at 16:12 -0800, Steve Ellcey wrote: > On Tue, 2017-11-21 at 17:35 +0000, James Greenhalgh wrote: > > > > > > > Thanks for the detailed explanation. I understood this, and my > > opinion is > > that the AArch64 parts of this patch are OK (and I don't know who > > needs to > > Ack the small generic changes you require). > > > > Let's give Richard/Marcus 48 hours to object while we wait for an > > OK on the > > generic bits, and then OK for AArch64. > > > > Thanks, > > James > > > > Reviewed-By: James Greenhalgh <james.greenhalgh@arm.com> > James, I haven't seen anything from Richard or Marcus.  Do you think > a > review from a global maintainer is really needed?  There is no > libatomic specific maintainer.  The only non-aarch64 specific change > is using the macro IFUNC_RESOLVER_ARGS in place of the hardcoded > 'void' > argument for ifunc selector functions and for all non-aarch64 > platforms > the macro will be defined as 'void' so there is no real change for > any > other platform. > > Steve Ellcey > sellcey@cavium.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-09-29 20:29 ` Steve Ellcey 2017-10-02 14:38 ` Szabolcs Nagy @ 2017-12-07 9:56 ` James Greenhalgh 2017-12-07 15:58 ` Steve Ellcey 1 sibling, 1 reply; 22+ messages in thread From: James Greenhalgh @ 2017-12-07 9:56 UTC (permalink / raw) To: Steve Ellcey; +Cc: Szabolcs Nagy, gcc-patches, nd On Fri, Sep 29, 2017 at 09:29:37PM +0100, Steve Ellcey wrote: > On Thu, 2017-09-28 at 12:31 +0100, Szabolcs Nagy wrote: > >Â > > i think this should be improved, see below. > > diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am > index d731406..92d19c6 100644 > --- a/libatomic/Makefile.am > +++ b/libatomic/Makefile.am > @@ -122,6 +122,10 @@ libatomic_la_LIBADD = $(foreach s,$(SIZES),$(addsuffix _$(s)_.lo,$(SIZEOBJS))) > > ## On a target-specific basis, include alternates to be selected by IFUNC. > if HAVE_IFUNC > +if ARCH_AARCH64_LINUX > +IFUNC_OPTIONS = -march=armv8.1-a > +libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS))) > +endif > if ARCH_ARM_LINUX > IFUNC_OPTIONS = -march=armv7-a -DHAVE_KERNEL64 > libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS))) One obvious thing I missed in the review is that this change will break bootstrap on systems with older assemblers. Practically, that's those of us who are holding out on Ubuntu 14.04. -march=armv8-a+lse would go back a little further, so would be preferable, but even this won't get bootstrap back on older systems. Is there anything you can do to check for assembler support before turning on IFUNCS for libatomic, or am I best to just start configuring with --disable-gnu-indirect-function ? Thanks, James ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 2017-12-07 9:56 ` James Greenhalgh @ 2017-12-07 15:58 ` Steve Ellcey 0 siblings, 0 replies; 22+ messages in thread From: Steve Ellcey @ 2017-12-07 15:58 UTC (permalink / raw) To: James Greenhalgh; +Cc: Szabolcs Nagy, gcc-patches, nd On Thu, 2017-12-07 at 09:56 +0000, James Greenhalgh wrote: >Â > One obvious thing I missed in the review is that this change will break > bootstrap on systems with older assemblers. Practically, that's those of > us who are holding out on Ubuntu 14.04. -march=armv8-a+lse would go back > a little further, so would be preferable, but even this won't get bootstrap > back on older systems. > > Is there anything you can do to check for assembler support before turning > on IFUNCS for libatomic, or am I best to just start configuring with > --disable-gnu-indirect-function ? > > Thanks, > James It should be possible to check for assembler support. Â I will work on a patch to do that. Steve Ellcey sellcey@cavium.com ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-12-07 15:58 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-07 20:44 [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64 Steve Ellcey 2017-08-07 20:46 ` Steve Ellcey 2017-08-25 4:56 ` Steve Ellcey 2017-08-25 16:43 ` Szabolcs Nagy 2017-08-28 18:40 ` Steve Ellcey 2017-08-29 11:42 ` Szabolcs Nagy 2017-08-30 18:39 ` Steve Ellcey 2017-08-31 18:55 ` Steve Ellcey 2017-09-27 20:35 ` Steve Ellcey 2017-09-28 11:31 ` Szabolcs Nagy 2017-09-29 20:29 ` Steve Ellcey 2017-10-02 14:38 ` Szabolcs Nagy 2017-10-03 18:57 ` Steve Ellcey 2017-10-24 18:17 ` Steve Ellcey 2017-11-20 18:27 ` Steve Ellcey 2017-11-20 18:29 ` James Greenhalgh 2017-11-20 19:50 ` Steve Ellcey 2017-11-21 17:36 ` James Greenhalgh 2017-11-29 8:09 ` Steve Ellcey 2017-12-05 0:51 ` Steve Ellcey 2017-12-07 9:56 ` James Greenhalgh 2017-12-07 15:58 ` Steve Ellcey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).