RE: [PATCH] libatomic: Improve ifunc selection on AArch64

[Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>]

Hi Wilco,

> -----Original Message-----
> From: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
> Sent: Monday, November 6, 2023 12:13 PM
> To: GCC Patches <gcc-patches@gcc.gnu.org>; Richard Sandiford
> <Richard.Sandiford@arm.com>
> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> Subject: Re: [PATCH] libatomic: Improve ifunc selection on AArch64
> 
> 
> 
> ping
> 
> 
> From: Wilco Dijkstra
> Sent: 04 August 2023 16:05
> To: GCC Patches <gcc-patches@gcc.gnu.org>; Richard Sandiford
> <Richard.Sandiford@arm.com>
> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> Subject: [PATCH] libatomic: Improve ifunc selection on AArch64
> 
> 
> Add support for ifunc selection based on CPUID register.  Neoverse N1
> supports
> atomic 128-bit load/store, so use the FEAT_USCAT ifunc like newer Neoverse
> cores.
> 
> Passes regress, OK for commit?
> 
> libatomic/
>         config/linux/aarch64/host-config.h (ifunc1): Use CPUID in ifunc
>         selection.
> 
> ---
> 
> diff --git a/libatomic/config/linux/aarch64/host-config.h
> b/libatomic/config/linux/aarch64/host-config.h
> index
> 851c78c01cd643318aaa52929ce4550266238b79..e5dc33c030a4bab927874fa6
> c69425db463fdc4b 100644
> --- a/libatomic/config/linux/aarch64/host-config.h
> +++ b/libatomic/config/linux/aarch64/host-config.h
> @@ -26,7 +26,7 @@
> 
>  #ifdef HWCAP_USCAT
>  # if N == 16
> -#  define IFUNC_COND_1 (hwcap & HWCAP_USCAT)
> +#  define IFUNC_COND_1 ifunc1 (hwcap)
>  # else
>  #  define IFUNC_COND_1  (hwcap & HWCAP_ATOMICS)
>  # endif
> @@ -50,4 +50,28 @@
>  #undef MAYBE_HAVE_ATOMIC_EXCHANGE_16
>  #define MAYBE_HAVE_ATOMIC_EXCHANGE_16   1
> 
> +#ifdef HWCAP_USCAT
> +
> +#define MIDR_IMPLEMENTOR(midr) (((midr) >> 24) & 255)
> +#define MIDR_PARTNUM(midr)     (((midr) >> 4) & 0xfff)
> +
> +static inline bool
> +ifunc1 (unsigned long hwcap)
> +{
> +  if (hwcap & HWCAP_USCAT)
> +    return true;
> +  if (!(hwcap & HWCAP_CPUID))
> +    return false;
> +
> +  unsigned long midr;
> +  asm volatile ("mrs %0, midr_el1" : "=r" (midr));

From what I recall that midr_el1 register is emulated by the kernel and so userspace software has to check that the kernel supports that emulation through hwcaps before reading it.
According to https://www.kernel.org/doc/html/v5.8/arm64/cpu-feature-registers.html you need to check getauxval(AT_HWCAP) & HWCAP_CPUID) before doing that read.

Thanks,
Kyrill

> +
> +  /* Neoverse N1 supports atomic 128-bit load/store.  */
> +  if (MIDR_IMPLEMENTOR (midr) == 'A' && MIDR_PARTNUM(midr) == 0xd0c)
> +    return true;
> +
> +  return false;
> +}
> +#endif
> +
>  #include_next <host-config.h>