[PATCH] AArch64: Update A64FX memset not to degrade at 16KB

[PATCH] AArch64: Update A64FX memset not to degrade at 16KB

[Naohiro Tamura]

This patch updates unroll8 code so as not to degrade at the peak
performance 16KB for both FX1000 and FX700.

Inserted 2 instructions at the beginning of the unroll8 loop,
cmp and branch, are a workaround that is found heuristically.

Reviewed-by: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
---
 sysdeps/aarch64/multiarch/memset_a64fx.S | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/sysdeps/aarch64/multiarch/memset_a64fx.S b/sysdeps/aarch64/multiarch/memset_a64fx.S
index 7bf759b6a753..f7dfdaace7cf 100644
--- a/sysdeps/aarch64/multiarch/memset_a64fx.S
+++ b/sysdeps/aarch64/multiarch/memset_a64fx.S
@@ -96,7 +96,14 @@ L(vl_agnostic): // VL Agnostic
 L(unroll8):
 	sub	count, count, tmp1
 	.p2align 4
-1:	st1b_unroll 0, 7
+	// The 2 instructions at the beginning of the following loop,
+	// cmp and branch, are a workaround so as not to degrade at
+	// the peak performance 16KB.
+	// It is found heuristically and the branch condition, b.ne,
+	// is chosen intentionally never to jump.
+1:	cmp	xzr, xzr
+	b.ne	1b
+	st1b_unroll 0, 7
 	add	dst, dst, tmp1
 	subs	count, count, tmp1
 	b.hi	1b
-- 
2.17.1

Re: [PATCH] AArch64: Update A64FX memset not to degrade at 16KB

[Szabolcs Nagy]

The 08/27/2021 05:03, Naohiro Tamura via Libc-alpha wrote:
> This patch updates unroll8 code so as not to degrade at the peak
> performance 16KB for both FX1000 and FX700.
> 
> Inserted 2 instructions at the beginning of the unroll8 loop,
> cmp and branch, are a workaround that is found heuristically.
> 
> Reviewed-by: Wilco Dijkstra <Wilco.Dijkstra@arm.com>

thanks, i committed this now.


> ---
>  sysdeps/aarch64/multiarch/memset_a64fx.S | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/sysdeps/aarch64/multiarch/memset_a64fx.S b/sysdeps/aarch64/multiarch/memset_a64fx.S
> index 7bf759b6a753..f7dfdaace7cf 100644
> --- a/sysdeps/aarch64/multiarch/memset_a64fx.S
> +++ b/sysdeps/aarch64/multiarch/memset_a64fx.S
> @@ -96,7 +96,14 @@ L(vl_agnostic): // VL Agnostic
>  L(unroll8):
>  	sub	count, count, tmp1
>  	.p2align 4
> -1:	st1b_unroll 0, 7
> +	// The 2 instructions at the beginning of the following loop,
> +	// cmp and branch, are a workaround so as not to degrade at
> +	// the peak performance 16KB.
> +	// It is found heuristically and the branch condition, b.ne,
> +	// is chosen intentionally never to jump.
> +1:	cmp	xzr, xzr
> +	b.ne	1b
> +	st1b_unroll 0, 7
>  	add	dst, dst, tmp1
>  	subs	count, count, tmp1
>  	b.hi	1b
> -- 
> 2.17.1
> 

Re: [PATCH] AArch64: Update A64FX memset not to degrade at 16KB

[naohirot]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 1663 bytes --]

Hi Szabolcs,

Thanks!

Naohiro
________________________________________
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Sent: Saturday, 4 September 2021 00:01
To: Tamura, Naohiro/Ìï´å Ö±ŽÚ
Cc: Wilco Dijkstra; libc-alpha@sourceware.org
Subject: Re: [PATCH] AArch64: Update A64FX memset not to degrade at 16KB

The 08/27/2021 05:03, Naohiro Tamura via Libc-alpha wrote:
> This patch updates unroll8 code so as not to degrade at the peak
> performance 16KB for both FX1000 and FX700.
>
> Inserted 2 instructions at the beginning of the unroll8 loop,
> cmp and branch, are a workaround that is found heuristically.
>
> Reviewed-by: Wilco Dijkstra <Wilco.Dijkstra@arm.com>

thanks, i committed this now.


> ---
>  sysdeps/aarch64/multiarch/memset_a64fx.S | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/aarch64/multiarch/memset_a64fx.S b/sysdeps/aarch64/multiarch/memset_a64fx.S
> index 7bf759b6a753..f7dfdaace7cf 100644
> --- a/sysdeps/aarch64/multiarch/memset_a64fx.S
> +++ b/sysdeps/aarch64/multiarch/memset_a64fx.S
> @@ -96,7 +96,14 @@ L(vl_agnostic): // VL Agnostic
>  L(unroll8):
>       sub     count, count, tmp1
>       .p2align 4
> -1:   st1b_unroll 0, 7
> +     // The 2 instructions at the beginning of the following loop,
> +     // cmp and branch, are a workaround so as not to degrade at
> +     // the peak performance 16KB.
> +     // It is found heuristically and the branch condition, b.ne,
> +     // is chosen intentionally never to jump.
> +1:   cmp     xzr, xzr
> +     b.ne    1b
> +     st1b_unroll 0, 7
>       add     dst, dst, tmp1
>       subs    count, count, tmp1
>       b.hi    1b
> --
> 2.17.1
>

Re: [PATCH] AArch64: Update A64FX memset not to degrade at 16KB

[naohirot]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 2235 bytes --]

Hi Szabolcs,

I noticed that my email address is not right in the commit log,
would you please fix it to <naohirot@fujitsu.com>?

commit 23777232c23f80809613bdfa329f63aadf992922
Author: Naohiro Tamura via Libc-alpha <libc-alpha@sourceware.org>
Date:   Fri Aug 27 05:03:04 2021 +0000

Thanks.
Naohiro

________________________________________
From: Tamura, Naohiro/Ìï´å Ö±ŽÚ <naohirot@fujitsu.com>
Sent: Monday, 6 September 2021 08:51
To: Szabolcs Nagy
Cc: Wilco Dijkstra; libc-alpha@sourceware.org
Subject: Re: [PATCH] AArch64: Update A64FX memset not to degrade at 16KB

Hi Szabolcs,

Thanks!

Naohiro
________________________________________
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Sent: Saturday, 4 September 2021 00:01
To: Tamura, Naohiro/Ìï´å Ö±ŽÚ
Cc: Wilco Dijkstra; libc-alpha@sourceware.org
Subject: Re: [PATCH] AArch64: Update A64FX memset not to degrade at 16KB

The 08/27/2021 05:03, Naohiro Tamura via Libc-alpha wrote:
> This patch updates unroll8 code so as not to degrade at the peak
> performance 16KB for both FX1000 and FX700.
>
> Inserted 2 instructions at the beginning of the unroll8 loop,
> cmp and branch, are a workaround that is found heuristically.
>
> Reviewed-by: Wilco Dijkstra <Wilco.Dijkstra@arm.com>

thanks, i committed this now.


> ---
>  sysdeps/aarch64/multiarch/memset_a64fx.S | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/aarch64/multiarch/memset_a64fx.S b/sysdeps/aarch64/multiarch/memset_a64fx.S
> index 7bf759b6a753..f7dfdaace7cf 100644
> --- a/sysdeps/aarch64/multiarch/memset_a64fx.S
> +++ b/sysdeps/aarch64/multiarch/memset_a64fx.S
> @@ -96,7 +96,14 @@ L(vl_agnostic): // VL Agnostic
>  L(unroll8):
>       sub     count, count, tmp1
>       .p2align 4
> -1:   st1b_unroll 0, 7
> +     // The 2 instructions at the beginning of the following loop,
> +     // cmp and branch, are a workaround so as not to degrade at
> +     // the peak performance 16KB.
> +     // It is found heuristically and the branch condition, b.ne,
> +     // is chosen intentionally never to jump.
> +1:   cmp     xzr, xzr
> +     b.ne    1b
> +     st1b_unroll 0, 7
>       add     dst, dst, tmp1
>       subs    count, count, tmp1
>       b.hi    1b
> --
> 2.17.1
>

Re: [PATCH] AArch64: Update A64FX memset not to degrade at 16KB

[Florian Weimer]

* naohirot:

> I noticed that my email address is not right in the commit log,
> would you please fix it to <naohirot@fujitsu.com>?
>
> commit 23777232c23f80809613bdfa329f63aadf992922
> Author: Naohiro Tamura via Libc-alpha <libc-alpha@sourceware.org>
> Date:   Fri Aug 27 05:03:04 2021 +0000

It's probably too late to do this now because it would mean rewriting
history.

Unfortunately, we do not have much control over the mailing list
configuration (to make “git am” work) or the commit hooks (to prevent
people from pushing such changes).  Sorry about that.

Florian

[PATCH] AArch64: Update A64FX memset not to degrade at 16KB

[naohirot]

Hi Florian, Szabolcs, and All maintainers,

> From: Florian Weimer <fweimer@redhat.com>
> Sent: Monday, September 6, 2021 3:18 PM
> * naohirot:
> 
> > I noticed that my email address is not right in the commit log,
> > would you please fix it to <naohirot@fujitsu.com>?
> >
> > commit 23777232c23f80809613bdfa329f63aadf992922
> > Author: Naohiro Tamura via Libc-alpha <libc-alpha@sourceware.org>
> > Date:   Fri Aug 27 05:03:04 2021 +0000
> 
> It's probably too late to do this now because it would mean rewriting
> history.

Yes, I know that it's impossible update only email address in the commit log.
Currently there are two commits after the patch, so rewriting history involves
only three records. 
If rewriting history is not acceptable, I prefer to revert and recommit.

"Author: " line in the commit log is the identity for each contributor.
And it's related to the " Update to glibc copyright assignment policy" discussion.
So I'd like to hear other maintainer's opinion.

Thanks.
Naohiro

Re: [PATCH] AArch64: Update A64FX memset not to degrade at 16KB

['Szabolcs Nagy']

The 09/06/2021 08:44, naohirot@fujitsu.com wrote:
> Hi Florian, Szabolcs, and All maintainers,
> 
> > From: Florian Weimer <fweimer@redhat.com>
> > Sent: Monday, September 6, 2021 3:18 PM
> > * naohirot:
> > 
> > > I noticed that my email address is not right in the commit log,
> > > would you please fix it to <naohirot@fujitsu.com>?
> > >
> > > commit 23777232c23f80809613bdfa329f63aadf992922
> > > Author: Naohiro Tamura via Libc-alpha <libc-alpha@sourceware.org>
> > > Date:   Fri Aug 27 05:03:04 2021 +0000
> > 
> > It's probably too late to do this now because it would mean rewriting
> > history.
> 
> Yes, I know that it's impossible update only email address in the commit log.
> Currently there are two commits after the patch, so rewriting history involves
> only three records. 
> If rewriting history is not acceptable, I prefer to revert and recommit.
> 
> "Author: " line in the commit log is the identity for each contributor.
> And it's related to the " Update to glibc copyright assignment policy" discussion.
> So I'd like to hear other maintainer's opinion.

sorry i didnt notice.
normally we don't fix such mistakes,
but i reverted and recommitted this now.

i really wish the mailing list setting was fixed
not to clobber email addresses by default.

RE: [PATCH] AArch64: Update A64FX memset not to degrade at 16KB

[naohirot]

Hi Szabolcs,

> From: 'Szabolcs Nagy' <szabolcs.nagy@arm.com>
> Sent: Monday, September 6, 2021 7:21 PM
>
> sorry i didnt notice.
> normally we don't fix such mistakes,
> but i reverted and recommitted this now.

Thank you very much!
 
> i really wish the mailing list setting was fixed
> not to clobber email addresses by default.

Yes, I think so too.

Thanks.
Naohiro

Re: [PATCH] AArch64: Update A64FX memset not to degrade at 16KB

[Joseph Myers]

On Mon, 6 Sep 2021, Florian Weimer via Libc-alpha wrote:

> Unfortunately, we do not have much control over the mailing list
> configuration (to make “git am” work) or the commit hooks (to prevent
> people from pushing such changes).  Sorry about that.

We do have sufficient control over the git hooks.  You can set 
commit-extra-checker (in refs/meta/config:project.config) to point to a 
script that applies such checks to commits.  See 
/git/gcc.git/hooks-bin/commit_checker for example; that includes code:

    # Reject commits applied via "git am" with list email address as
    # the author.
    if author_email in ('gcc-patches@gcc.gnu.org', 'libstdc++@gcc.gnu.org',
                        'fortran@gcc.gnu.org'):
        error('When applying a patch from a mailing list, make sure '
              'to use the original name and email address of the patch '
              'author, not the list email address with "via Gcc-patches".')

These checks are only applied to commits new to the repository, not 
commits new to a particular branch but already present in the repository, 
so applying stricter checks to new commits on particular branches can be 
trickier (see <https://github.com/AdaCore/git-hooks/issues/20>), but for 
this particular issue a branch-independent check should suffice.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] AArch64: Update A64FX memset not to degrade at 16KB

[Florian Weimer]

* Joseph Myers:

> On Mon, 6 Sep 2021, Florian Weimer via Libc-alpha wrote:
>
>> Unfortunately, we do not have much control over the mailing list
>> configuration (to make “git am” work) or the commit hooks (to prevent
>> people from pushing such changes).  Sorry about that.
>
> We do have sufficient control over the git hooks.  You can set 
> commit-extra-checker (in refs/meta/config:project.config) to point to a 
> script that applies such checks to commits.  See 
> /git/gcc.git/hooks-bin/commit_checker for example; that includes code:
>
>     # Reject commits applied via "git am" with list email address as
>     # the author.
>     if author_email in ('gcc-patches@gcc.gnu.org', 'libstdc++@gcc.gnu.org',
>                         'fortran@gcc.gnu.org'):
>         error('When applying a patch from a mailing list, make sure '
>               'to use the original name and email address of the patch '
>               'author, not the list email address with "via Gcc-patches".')
>
> These checks are only applied to commits new to the repository, not 
> commits new to a particular branch but already present in the repository, 
> so applying stricter checks to new commits on particular branches can be 
> trickier (see <https://github.com/AdaCore/git-hooks/issues/20>), but for 
> this particular issue a branch-independent check should suffice.

I've tried to implement this now.  commit-extra-checker now points to a
trimmed-down version of the GCC script.

Thanks,
Florian

RE: [PATCH] AArch64: Update A64FX memset not to degrade at 16KB

[naohirot]

Hi Florian, Joseph,

> > These checks are only applied to commits new to the repository, not
> > commits new to a particular branch but already present in the repository,
> > so applying stricter checks to new commits on particular branches can be
> > trickier (see <https://github.com/AdaCore/git-hooks/issues/20>), but for
> > this particular issue a branch-independent check should suffice.
> 
> I've tried to implement this now.  commit-extra-checker now points to a
> trimmed-down version of the GCC script.

That's great!
Thank you for the update.
Naohiro