* [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).