Re: [PATCH] libatomic: Fix build for --disable-gnu-indirect-function [PR113986]

[Richard Sandiford <richard.sandiford@arm.com>]

Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> Fix libatomic build to support --disable-gnu-indirect-function on AArch64.
> Always build atomic_16.S and add aliases to the __atomic_* functions if
> !HAVE_IFUNC.

This description is too brief for me.  Could you say in detail how the
new scheme works?  E.g. the description doesn't explain:

> -if ARCH_AARCH64_HAVE_LSE128
> -AM_CPPFLAGS	     = -DHAVE_FEAT_LSE128
> -endif

And what's the purpose of ARCH_AARCH64_HAVE_LSE128 after this change?

Is the indirection via ALIAS2 necessary?  Couldn't ENTRY just define
the __atomic_* symbols directly, as non-hidden, if we remove the
libat_ prefix?  That would make it easier to ensure that the lists
are kept up-to-date.

Shouldn't we skip the ENTRY_FEAT functions and existing aliases
if !HAVE_IFUNC?

I think it'd be worth (as a prepatch) splitting the file into two
#included subfiles, one that contains the base AArch64 routines and one
that contains the optimised versions.  The former would then be #included
for all builds while the latter would be specific to HAVE_IFUNC.

Thanks,
Richard

> Passes regress and bootstrap, OK for commit?
>
> libatomic:
>         PR target/113986
>         * Makefile.in: Regenerated.
>         * Makefile.am: Make atomic_16.S not depend on HAVE_IFUNC.
>         Remove predefine of HAVE_FEAT_LSE128.
>         * config/linux/aarch64/atomic_16.S: Add __atomic_ aliases if !HAVE_IFUNC.	
>         * config/linux/aarch64/host-config.h: Correctly handle !HAVE_IFUNC.
>
> ---
>
> diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am
> index d49c44c7d5fbe83061fddd1f8ef4813a39eb1b8b..980677f353345c050f6cef2d57090360216c56cf 100644
> --- a/libatomic/Makefile.am
> +++ b/libatomic/Makefile.am
> @@ -130,12 +130,8 @@ 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
> -if ARCH_AARCH64_HAVE_LSE128
> -AM_CPPFLAGS	     = -DHAVE_FEAT_LSE128
> -endif
>  IFUNC_OPTIONS	     = -march=armv8-a+lse
>  libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS)))
> -libatomic_la_SOURCES += atomic_16.S
>  
>  endif
>  if ARCH_ARM_LINUX
> @@ -155,6 +151,10 @@ libatomic_la_LIBADD += $(addsuffix _16_1_.lo,$(SIZEOBJS)) \
>  endif
>  endif
>  
> +if ARCH_AARCH64_LINUX
> +libatomic_la_SOURCES += atomic_16.S
> +endif
> +
>  libatomic_convenience_la_SOURCES = $(libatomic_la_SOURCES)
>  libatomic_convenience_la_LIBADD = $(libatomic_la_LIBADD)
>  
> diff --git a/libatomic/Makefile.in b/libatomic/Makefile.in
> index 11c8ec7ba15ba7da5ef55e90bd836317bc270061..d9d529bc502d4ce7b9997640d5f40f5d5cc1232c 100644
> --- a/libatomic/Makefile.in
> +++ b/libatomic/Makefile.in
> @@ -90,17 +90,17 @@ build_triplet = @build@
>  host_triplet = @host@
>  target_triplet = @target@
>  @ARCH_AARCH64_LINUX_TRUE@@HAVE_IFUNC_TRUE@am__append_1 = $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS)))
> -@ARCH_AARCH64_LINUX_TRUE@@HAVE_IFUNC_TRUE@am__append_2 = atomic_16.S
> -@ARCH_ARM_LINUX_TRUE@@HAVE_IFUNC_TRUE@am__append_3 = $(foreach \
> +@ARCH_ARM_LINUX_TRUE@@HAVE_IFUNC_TRUE@am__append_2 = $(foreach \
>  @ARCH_ARM_LINUX_TRUE@@HAVE_IFUNC_TRUE@	s,$(SIZES),$(addsuffix \
>  @ARCH_ARM_LINUX_TRUE@@HAVE_IFUNC_TRUE@	_$(s)_1_.lo,$(SIZEOBJS))) \
>  @ARCH_ARM_LINUX_TRUE@@HAVE_IFUNC_TRUE@	$(addsuffix \
>  @ARCH_ARM_LINUX_TRUE@@HAVE_IFUNC_TRUE@	_8_2_.lo,$(SIZEOBJS)) \
>  @ARCH_ARM_LINUX_TRUE@@HAVE_IFUNC_TRUE@	tas_1_2_.lo
> -@ARCH_I386_TRUE@@HAVE_IFUNC_TRUE@am__append_4 = $(addsuffix _8_1_.lo,$(SIZEOBJS))
> -@ARCH_X86_64_TRUE@@HAVE_IFUNC_TRUE@am__append_5 = $(addsuffix _16_1_.lo,$(SIZEOBJS)) \
> +@ARCH_I386_TRUE@@HAVE_IFUNC_TRUE@am__append_3 = $(addsuffix _8_1_.lo,$(SIZEOBJS))
> +@ARCH_X86_64_TRUE@@HAVE_IFUNC_TRUE@am__append_4 = $(addsuffix _16_1_.lo,$(SIZEOBJS)) \
>  @ARCH_X86_64_TRUE@@HAVE_IFUNC_TRUE@		       $(addsuffix _16_2_.lo,$(SIZEOBJS))
>  
> +@ARCH_AARCH64_LINUX_TRUE@am__append_5 = atomic_16.S
>  subdir = .
>  ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
>  am__aclocal_m4_deps = $(top_srcdir)/../config/acx.m4 \
> @@ -156,8 +156,7 @@ am__uninstall_files_from_dir = { \
>    }
>  am__installdirs = "$(DESTDIR)$(toolexeclibdir)"
>  LTLIBRARIES = $(noinst_LTLIBRARIES) $(toolexeclib_LTLIBRARIES)
> -@ARCH_AARCH64_LINUX_TRUE@@HAVE_IFUNC_TRUE@am__objects_1 =  \
> -@ARCH_AARCH64_LINUX_TRUE@@HAVE_IFUNC_TRUE@	atomic_16.lo
> +@ARCH_AARCH64_LINUX_TRUE@am__objects_1 = atomic_16.lo
>  am_libatomic_la_OBJECTS = gload.lo gstore.lo gcas.lo gexch.lo \
>  	glfree.lo lock.lo init.lo fenv.lo fence.lo flag.lo \
>  	$(am__objects_1)
> @@ -425,7 +424,7 @@ libatomic_la_LDFLAGS = $(libatomic_version_info) $(libatomic_version_script) \
>  	$(lt_host_flags) $(libatomic_darwin_rpath)
>  
>  libatomic_la_SOURCES = gload.c gstore.c gcas.c gexch.c glfree.c lock.c \
> -	init.c fenv.c fence.c flag.c $(am__append_2)
> +	init.c fenv.c fence.c flag.c $(am__append_5)
>  SIZEOBJS = load store cas exch fadd fsub fand fior fxor fnand tas
>  EXTRA_libatomic_la_SOURCES = $(addsuffix _n.c,$(SIZEOBJS))
>  libatomic_la_DEPENDENCIES = $(libatomic_la_LIBADD) $(libatomic_version_dep)
> @@ -451,9 +450,8 @@ all_c_files := $(foreach dir,$(search_path),$(wildcard $(dir)/*.c))
>  # Then sort through them to find the one we want, and select the first.
>  M_SRC = $(firstword $(filter %/$(M_FILE), $(all_c_files)))
>  libatomic_la_LIBADD = $(foreach s,$(SIZES),$(addsuffix \
> -	_$(s)_.lo,$(SIZEOBJS))) $(am__append_1) $(am__append_3) \
> -	$(am__append_4) $(am__append_5)
> -@ARCH_AARCH64_HAVE_LSE128_TRUE@@ARCH_AARCH64_LINUX_TRUE@@HAVE_IFUNC_TRUE@AM_CPPFLAGS = -DHAVE_FEAT_LSE128
> +	_$(s)_.lo,$(SIZEOBJS))) $(am__append_1) $(am__append_2) \
> +	$(am__append_3) $(am__append_4)
>  @ARCH_AARCH64_LINUX_TRUE@@HAVE_IFUNC_TRUE@IFUNC_OPTIONS = -march=armv8-a+lse
>  @ARCH_ARM_LINUX_TRUE@@HAVE_IFUNC_TRUE@IFUNC_OPTIONS = -march=armv7-a+fp -DHAVE_KERNEL64
>  @ARCH_I386_TRUE@@HAVE_IFUNC_TRUE@IFUNC_OPTIONS = -march=i586
> diff --git a/libatomic/config/linux/aarch64/atomic_16.S b/libatomic/config/linux/aarch64/atomic_16.S
> index d4a360a6f7812351249d0a0ad7f60373b7f8c35a..772f84c0a24dab52773e25d053f5006fa8f28560 100644
> --- a/libatomic/config/linux/aarch64/atomic_16.S
> +++ b/libatomic/config/linux/aarch64/atomic_16.S
> @@ -38,6 +38,8 @@
>     The libat_<op>_16_i1 entry points are used when LSE128 is available.
>     The libat_<op>_16_i2 entry points are used when LSE2 is available.  */
>  
> +#include "auto-config.h"
> +
>  #if HAVE_FEAT_LSE128
>  	.arch	armv9-a+lse128
>  #else
> @@ -67,8 +69,8 @@ NAME:				\
>  	.cfi_endproc;		\
>  	.size NAME, .-NAME;
>  
> -#define ALIAS(NAME, FROM, TO)	\
> -	ALIAS1 (FROM (NAME),TO (NAME))
> +#define ALIAS(NAME, FROM, TO)	ALIAS1 (FROM (NAME),TO (NAME))
> +#define ALIAS2(NAME)		ALIAS1 (__atomic_##NAME, libat_##NAME)
>  
>  #define ALIAS1(ALIAS, NAME)	\
>  	.global ALIAS;		\
> @@ -747,6 +749,28 @@ ALIAS (libat_fetch_nand_16, LSE2, CORE)
>  ALIAS (libat_nand_fetch_16, LSE2, CORE)
>  ALIAS (libat_test_and_set_16, LSE2, CORE)
>  
> +/* Emit __atomic_* entrypoints if no ifuncs.  */
> +
> +#if !HAVE_IFUNC
> +ALIAS2 (load_16)
> +ALIAS2 (store_16)
> +ALIAS2 (compare_exchange_16)
> +ALIAS2 (exchange_16)
> +ALIAS2 (fetch_add_16)
> +ALIAS2 (add_fetch_16)
> +ALIAS2 (fetch_sub_16)
> +ALIAS2 (sub_fetch_16)
> +ALIAS2 (fetch_or_16)
> +ALIAS2 (or_fetch_16)
> +ALIAS2 (fetch_and_16)
> +ALIAS2 (and_fetch_16)
> +ALIAS2 (fetch_xor_16)
> +ALIAS2 (xor_fetch_16)
> +ALIAS2 (fetch_nand_16)
> +ALIAS2 (nand_fetch_16)
> +ALIAS2 (test_and_set_16)
> +#endif
> +
>  /* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code.  */
>  #define FEATURE_1_AND 0xc0000000
>  #define FEATURE_1_BTI 1
> diff --git a/libatomic/config/linux/aarch64/host-config.h b/libatomic/config/linux/aarch64/host-config.h
> index 4e3541240633dc26de4a57c506b7e4b0c50185c2..030b56ae4df97f1fd33f141c956d16d5eafb89e1 100644
> --- a/libatomic/config/linux/aarch64/host-config.h
> +++ b/libatomic/config/linux/aarch64/host-config.h
> @@ -24,6 +24,13 @@
>  #if HAVE_IFUNC
>  #include <sys/auxv.h>
>  
> +#ifndef HWCAP_USCAT
> +# define HWCAP_USCAT	(1 << 25)
> +#endif
> +#ifndef HWCAP2_LSE128
> +# define HWCAP2_LSE128	(1UL << 47)
> +#endif
> +
>  #if __has_include(<sys/ifunc.h>)
>  # include <sys/ifunc.h>
>  #else
> @@ -35,7 +42,6 @@ typedef struct __ifunc_arg_t {
>  # define _IFUNC_ARG_HWCAP (1ULL << 62)
>  #endif
>  
> -#ifdef HWCAP_USCAT
>  # if N == 16
>  #  define IFUNC_COND_1		(has_lse128 (hwcap, features))
>  #  define IFUNC_COND_2		(has_lse2 (hwcap, features))
> @@ -44,19 +50,6 @@ typedef struct __ifunc_arg_t {
>  #  define IFUNC_COND_1		(hwcap & HWCAP_ATOMICS)
>  #  define IFUNC_NCOND(N)	1
>  # endif
> -#else
> -#  define IFUNC_COND_1	(false)
> -#  define IFUNC_NCOND(N)	1
> -#endif
> -
> -#endif /* HAVE_IFUNC */
> -
> -/* All 128-bit atomic functions are defined in aarch64/atomic_16.S.  */
> -#if N == 16
> -# define DONE 1
> -#endif
> -
> -#ifdef HWCAP_USCAT
>  
>  #define MIDR_IMPLEMENTOR(midr)	(((midr) >> 24) & 255)
>  #define MIDR_PARTNUM(midr)	(((midr) >> 4) & 0xfff)
> @@ -89,11 +82,6 @@ has_lse2 (unsigned long hwcap, const __ifunc_arg_t *features)
>  
>  #define AT_FEAT_FIELD(isar0)	(((isar0) >> 20) & 15)
>  
> -/* Ensure backwards compatibility with glibc <= 2.38.  */
> -#ifndef HWCAP2_LSE128
> -#define HWCAP2_LSE128		(1UL << 47)
> -#endif
> -
>  static inline bool
>  has_lse128 (unsigned long hwcap, const __ifunc_arg_t *features)
>  {
> @@ -116,6 +104,14 @@ has_lse128 (unsigned long hwcap, const __ifunc_arg_t *features)
>    return false;
>  }
>  
> +#endif /* HAVE_IFUNC */
> +
> +/* All 128-bit atomic functions are defined in aarch64/atomic_16.S.  */
> +#if N == 16
> +# define DONE 1
> +# if !HAVE_IFUNC
> +#  define IFUNC_ALT 1
> +# endif
>  #endif
>  
>  #include_next <host-config.h>