unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Pinski <pinskia@gmail.com>
To: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
Cc: Siddhesh Poyarekar <siddhesh@gotplt.org>,
	GNU C Library <libc-alpha@sourceware.org>
Subject: Re: [PATCH][AArch64] Cleanup memset
Date: Tue, 3 Mar 2020 11:34:34 -0800	[thread overview]
Message-ID: <CA+=Sn1kTESbPq95gnT2CiCXgd73DbXgRvh3F1JWibo+jZ8B4JQ@mail.gmail.com> (raw)
In-Reply-To: <AM5PR0801MB2035C6F8F3E5DD25D5E540FF83E40@AM5PR0801MB2035.eurprd08.prod.outlook.com>

On Tue, Mar 3, 2020 at 6:42 AM Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>
> Hi Siddhesh,
>
> > This looks OK in general, although can you please elaborate on the
> > following:
> >
> > - What cores did you test on to conclude that 160 is a better threshold
> > than 256?
>
> I've mostly done testing on Neoverse N1, Cortex-A72 and Cortex-A53.
> (the latter seems to be always faster with DC ZVA disabled, so the threshold
> doesn't really matter). I wrote a random memset benchmark similar to the
> memcpy one, and performance is unchanged there given there is no change
> in the way small cases are handled.
>
> > - Is the intention to support non-64-byte zva sizes once there is actual
> > hardware that implements it and not bother with it for now?  I agree
> > with the idea if that's the case, just that it would be nice to have
> > that documented in the git commit message.
>
> Yes, otherwise it's hard to test or prove it helps performance after all. We've
> had issues with the non-64 ZVA sizes before, so it's best to keep it simple.
>
> I'm also trying to reduce the amount of code and avoid unnecessary proliferation
> of almost identical ifuncs. I think we can remove most of the memset ifuncs,
> it seems we need one version without ZVA and a ZVA version for size 64.

I am trying to understand what was the decision here.  The main reason
is OcteonTX2 does ZVA 128 which was faster than doing one without
(OcteonTX1 is similar but has an errata which causes ZVA to be turned
off).

Thanks,
Andrew Pinski

>
> Cheers,
> Wilco

  parent reply	other threads:[~2020-03-03 19:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-26 16:11 [PATCH][AArch64] Cleanup memset Wilco Dijkstra
2020-02-27  5:50 ` Siddhesh Poyarekar
2020-03-03 14:42   ` Wilco Dijkstra
2020-03-03 15:23     ` Siddhesh Poyarekar
2020-03-03 19:34     ` Andrew Pinski [this message]
2020-03-12 15:46       ` Wilco Dijkstra
2020-03-12 17:06         ` Andrew Pinski via Libc-alpha
2020-03-13  2:24           ` Siddhesh Poyarekar

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='CA+=Sn1kTESbPq95gnT2CiCXgd73DbXgRvh3F1JWibo+jZ8B4JQ@mail.gmail.com' \
    --to=pinskia@gmail.com \
    --cc=Wilco.Dijkstra@arm.com \
    --cc=libc-alpha@sourceware.org \
    --cc=siddhesh@gotplt.org \
    /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).