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
> sell...@cavium.com
>
>
> 2017-08-31 Steve Ellcey <sell...@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'
