unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Szabolcs Nagy via Libc-alpha <libc-alpha@sourceware.org>
To: Naohiro Tamura <naohirot@fujitsu.com>
Cc: Naohiro Tamura <naohirot@jp.fujitsu.com>, libc-alpha@sourceware.org
Subject: Re: [PATCH 2/5] aarch64: Added optimized memcpy and memmove for A64FX
Date: Mon, 29 Mar 2021 13:44:37 +0100	[thread overview]
Message-ID: <20210329124436.GE23289@arm.com> (raw)
In-Reply-To: <20210317023417.323152-1-naohirot@fujitsu.com>

The 03/17/2021 02:34, Naohiro Tamura wrote:
> And also we confirmed that the SVE 512 bit vector register performance
> is roughly 4 times better than Advanced SIMD 128 bit register and 8
> times better than scalar 64 bit register by running 'make bench'.

nice speed up. i won't comment on the memcpy asm now.

> diff --git a/manual/tunables.texi b/manual/tunables.texi
> index 1b746c0fa1..81ed5366fc 100644
> --- a/manual/tunables.texi
> +++ b/manual/tunables.texi
> @@ -453,7 +453,8 @@ This tunable is specific to powerpc, powerpc64 and powerpc64le.
>  The @code{glibc.cpu.name=xxx} tunable allows the user to tell @theglibc{} to
>  assume that the CPU is @code{xxx} where xxx may have one of these values:
>  @code{generic}, @code{falkor}, @code{thunderxt88}, @code{thunderx2t99},
> -@code{thunderx2t99p1}, @code{ares}, @code{emag}, @code{kunpeng}.
> +@code{thunderx2t99p1}, @code{ares}, @code{emag}, @code{kunpeng},
> +@code{a64fx}.

OK.

> --- a/sysdeps/aarch64/multiarch/Makefile
> +++ b/sysdeps/aarch64/multiarch/Makefile
> @@ -1,6 +1,6 @@
>  ifeq ($(subdir),string)
>  sysdep_routines += memcpy_generic memcpy_advsimd memcpy_thunderx memcpy_thunderx2 \
> -		   memcpy_falkor \
> +		   memcpy_falkor memcpy_a64fx \
>  		   memset_generic memset_falkor memset_emag memset_kunpeng \

OK.

> --- a/sysdeps/aarch64/multiarch/ifunc-impl-list.c
> +++ b/sysdeps/aarch64/multiarch/ifunc-impl-list.c
> @@ -25,7 +25,11 @@
>  #include <stdio.h>
>  
>  /* Maximum number of IFUNC implementations.  */
> -#define MAX_IFUNC	4
> +#if HAVE_SVE_ASM_SUPPORT
> +# define MAX_IFUNC	7
> +#else
> +# define MAX_IFUNC	6
> +#endif

hm this MAX_IFUNC looks a bit problematic: currently its only
use is to detect if a target requires more ifuncs than the
array passed to __libc_ifunc_impl_list, but for that ideally
it would be automatic, not manually maintained.

i would just define it to 7 unconditionally (the maximum over
valid configurations).

>  size_t
>  __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
> @@ -43,12 +47,18 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
>  	      IFUNC_IMPL_ADD (array, i, memcpy, !bti, __memcpy_thunderx2)
>  	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_falkor)
>  	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_simd)
> +#if HAVE_SVE_ASM_SUPPORT
> +	      IFUNC_IMPL_ADD (array, i, memcpy, sve, __memcpy_a64fx)
> +#endif

OK.

>  	      IFUNC_IMPL_ADD (array, i, memcpy, 1, __memcpy_generic))
>    IFUNC_IMPL (i, name, memmove,
>  	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_thunderx)
>  	      IFUNC_IMPL_ADD (array, i, memmove, !bti, __memmove_thunderx2)
>  	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_falkor)
>  	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_simd)
> +#if HAVE_SVE_ASM_SUPPORT
> +	      IFUNC_IMPL_ADD (array, i, memmove, sve, __memmove_a64fx)
> +#endif

OK.

>  	      IFUNC_IMPL_ADD (array, i, memmove, 1, __memmove_generic))
>    IFUNC_IMPL (i, name, memset,
>  	      /* Enable this on non-falkor processors too so that other cores
> diff --git a/sysdeps/aarch64/multiarch/init-arch.h b/sysdeps/aarch64/multiarch/init-arch.h
> index a167699e74..d20e7e1b8e 100644
> --- a/sysdeps/aarch64/multiarch/init-arch.h
> +++ b/sysdeps/aarch64/multiarch/init-arch.h
> @@ -33,4 +33,6 @@
>    bool __attribute__((unused)) bti =					      \
>      HAVE_AARCH64_BTI && GLRO(dl_aarch64_cpu_features).bti;		      \
>    bool __attribute__((unused)) mte =					      \
> -    MTE_ENABLED ();
> +    MTE_ENABLED ();							      \
> +  unsigned __attribute__((unused)) sve =				      \
> +    GLRO(dl_aarch64_cpu_features).sve;

i would use bool here.

> diff --git a/sysdeps/aarch64/multiarch/memcpy.c b/sysdeps/aarch64/multiarch/memcpy.c
> index 0e0a5cbcfb..0006f38eb0 100644
> --- a/sysdeps/aarch64/multiarch/memcpy.c
> +++ b/sysdeps/aarch64/multiarch/memcpy.c
> @@ -33,6 +33,9 @@ extern __typeof (__redirect_memcpy) __memcpy_simd attribute_hidden;
>  extern __typeof (__redirect_memcpy) __memcpy_thunderx attribute_hidden;
>  extern __typeof (__redirect_memcpy) __memcpy_thunderx2 attribute_hidden;
>  extern __typeof (__redirect_memcpy) __memcpy_falkor attribute_hidden;
> +#if HAVE_SVE_ASM_SUPPORT
> +extern __typeof (__redirect_memcpy) __memcpy_a64fx attribute_hidden;
> +#endif

OK.

>  libc_ifunc (__libc_memcpy,
>              (IS_THUNDERX (midr)
> @@ -44,8 +47,13 @@ libc_ifunc (__libc_memcpy,
>  		  : (IS_NEOVERSE_N1 (midr) || IS_NEOVERSE_N2 (midr)
>  		     || IS_NEOVERSE_V1 (midr)
>  		     ? __memcpy_simd
> -		     : __memcpy_generic)))));
> -
> +#if HAVE_SVE_ASM_SUPPORT
> +                     : (IS_A64FX (midr)
> +                        ? __memcpy_a64fx
> +                        : __memcpy_generic))))));
> +#else
> +                     : __memcpy_generic)))));
> +#endif

OK.

> new file mode 100644
> index 0000000000..23438e4e3d
> --- /dev/null
> +++ b/sysdeps/aarch64/multiarch/memcpy_a64fx.S

skipping this.

> diff --git a/sysdeps/aarch64/multiarch/memmove.c b/sysdeps/aarch64/multiarch/memmove.c
> index 12d77818a9..1e5ee1c934 100644
> --- a/sysdeps/aarch64/multiarch/memmove.c
> +++ b/sysdeps/aarch64/multiarch/memmove.c
> @@ -33,6 +33,9 @@ extern __typeof (__redirect_memmove) __memmove_simd attribute_hidden;
>  extern __typeof (__redirect_memmove) __memmove_thunderx attribute_hidden;
>  extern __typeof (__redirect_memmove) __memmove_thunderx2 attribute_hidden;
>  extern __typeof (__redirect_memmove) __memmove_falkor attribute_hidden;
> +#if HAVE_SVE_ASM_SUPPORT
> +extern __typeof (__redirect_memmove) __memmove_a64fx attribute_hidden;
> +#endif

OK.

>  
>  libc_ifunc (__libc_memmove,
>              (IS_THUNDERX (midr)
> @@ -44,8 +47,13 @@ libc_ifunc (__libc_memmove,
>  		  : (IS_NEOVERSE_N1 (midr) || IS_NEOVERSE_N2 (midr)
>  		     || IS_NEOVERSE_V1 (midr)
>  		     ? __memmove_simd
> -		     : __memmove_generic)))));
> -
> +#if HAVE_SVE_ASM_SUPPORT
> +                     : (IS_A64FX (midr)
> +                        ? __memmove_a64fx
> +                        : __memmove_generic))))));
> +#else
> +                        : __memmove_generic)))));
> +#endif

OK.

> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> index db6aa3516c..6206a2f618 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> @@ -46,6 +46,7 @@ static struct cpu_list cpu_list[] = {
>        {"ares",		 0x411FD0C0},
>        {"emag",		 0x503F0001},
>        {"kunpeng920", 	 0x481FD010},
> +      {"a64fx",		 0x460F0010},
>        {"generic", 	 0x0}

OK.

> +
> +  /* Check if SVE is supported.  */
> +  cpu_features->sve = GLRO (dl_hwcap) & HWCAP_SVE;

OK.

>  }
> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> index 3b9bfed134..2b322e5414 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> @@ -65,6 +65,9 @@
>  #define IS_KUNPENG920(midr) (MIDR_IMPLEMENTOR(midr) == 'H'			   \
>                          && MIDR_PARTNUM(midr) == 0xd01)
>  
> +#define IS_A64FX(midr) (MIDR_IMPLEMENTOR(midr) == 'F'			      \
> +			&& MIDR_PARTNUM(midr) == 0x001)
> +

OK.

>  struct cpu_features
>  {
>    uint64_t midr_el1;
> @@ -72,6 +75,7 @@ struct cpu_features
>    bool bti;
>    /* Currently, the GLIBC memory tagging tunable only defines 8 bits.  */
>    uint8_t mte_state;
> +  bool sve;
>  };

OK.


  reply	other threads:[~2021-03-29 12:44 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-17  2:28 [PATCH 0/5] Added optimized memcpy/memmove/memset for A64FX Naohiro Tamura
2021-03-17  2:33 ` [PATCH 1/5] config: Added HAVE_SVE_ASM_SUPPORT for aarch64 Naohiro Tamura
2021-03-29 12:11   ` Szabolcs Nagy via Libc-alpha
2021-03-30  6:19     ` naohirot
2021-03-17  2:34 ` [PATCH 2/5] aarch64: Added optimized memcpy and memmove for A64FX Naohiro Tamura
2021-03-29 12:44   ` Szabolcs Nagy via Libc-alpha [this message]
2021-03-30  7:17     ` naohirot
2021-03-17  2:34 ` [PATCH 3/5] aarch64: Added optimized memset " Naohiro Tamura
2021-03-17  2:35 ` [PATCH 4/5] scripts: Added Vector Length Set test helper script Naohiro Tamura
2021-03-29 13:20   ` Szabolcs Nagy via Libc-alpha
2021-03-30  7:25     ` naohirot
2021-03-17  2:35 ` [PATCH 5/5] benchtests: Added generic_memcpy and generic_memmove to large benchtests Naohiro Tamura
2021-03-29 12:03 ` [PATCH 0/5] Added optimized memcpy/memmove/memset for A64FX Szabolcs Nagy via Libc-alpha
2021-05-10  1:45 ` naohirot
2021-05-14 13:35   ` Szabolcs Nagy via Libc-alpha
2021-05-19  0:11     ` naohirot
2021-05-12  9:23 ` [PATCH v2 0/6] aarch64: " Naohiro Tamura
2021-05-12  9:26   ` [PATCH v2 1/6] config: Added HAVE_AARCH64_SVE_ASM for aarch64 Naohiro Tamura
2021-05-26 10:05     ` Szabolcs Nagy via Libc-alpha
2021-05-12  9:27   ` [PATCH v2 2/6] aarch64: define BTI_C and BTI_J macros as NOP unless HAVE_AARCH64_BTI Naohiro Tamura
2021-05-26 10:06     ` Szabolcs Nagy via Libc-alpha
2021-05-12  9:28   ` [PATCH v2 3/6] aarch64: Added optimized memcpy and memmove for A64FX Naohiro Tamura
2021-05-26 10:19     ` Szabolcs Nagy via Libc-alpha
2021-05-12  9:28   ` [PATCH v2 4/6] aarch64: Added optimized memset " Naohiro Tamura
2021-05-26 10:22     ` Szabolcs Nagy via Libc-alpha
2021-05-12  9:29   ` [PATCH v2 5/6] scripts: Added Vector Length Set test helper script Naohiro Tamura
2021-05-12 16:58     ` Joseph Myers
2021-05-13  9:53       ` naohirot
2021-05-20  7:34     ` Naohiro Tamura
2021-05-26 10:24       ` Szabolcs Nagy via Libc-alpha
2021-05-12  9:29   ` [PATCH v2 6/6] benchtests: Fixed bench-memcpy-random: buf1: mprotect failed Naohiro Tamura
2021-05-26 10:25     ` Szabolcs Nagy via Libc-alpha
2021-05-27  0:22   ` [PATCH v2 0/6] aarch64: Added optimized memcpy/memmove/memset for A64FX naohirot
2021-05-27 23:50     ` naohirot
2021-05-27  7:42   ` [PATCH v3 1/2] aarch64: Added optimized memcpy and memmove " Naohiro Tamura
2021-05-27  7:44   ` [PATCH v3 2/2] aarch64: Added optimized memset " Naohiro Tamura

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210329124436.GE23289@arm.com \
    --to=libc-alpha@sourceware.org \
    --cc=naohirot@fujitsu.com \
    --cc=naohirot@jp.fujitsu.com \
    --cc=szabolcs.nagy@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).