AArch64: Update A64FX memset not to degrade at 16KB

  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(-)
  

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
>
  

* 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
  

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
  

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.
  

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
  

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 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
  

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