[PATCH v2] x86: Add new cpu-flag `Prefer_Non_Temporal`

[Noah Goldstein]

On Tue, Aug 13, 2024 at 9:08 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Aug 12, 2024 at 12:45 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Mon, Aug 12, 2024 at 12:06 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> > >
> > > On Mon, Aug 12, 2024 at 7:38 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > On Sat, Aug 10, 2024 at 10:56 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> > > > >
> > > > > The goal of this flag is to allow targets which don't prefer/have ERMS
> > > > > to still access the non-temporal memset implementation.
> > > > > ---
> > > > >  sysdeps/x86/cpu-tunables.c                    |  2 ++
> > > > >  sysdeps/x86/dl-cacheinfo.h                    | 32 ++++++++++++++++---
> > > > >  ...cpu-features-preferred_feature_index_1.def |  1 +
> > > > >  sysdeps/x86/tst-hwcap-tunables.c              |  6 ++--
> > > > >  sysdeps/x86_64/multiarch/ifunc-memset.h       | 15 ++++++---
> > > > >  5 files changed, 45 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/sysdeps/x86/cpu-tunables.c b/sysdeps/x86/cpu-tunables.c
> > > > > index ccc6b64dc2..789646ba26 100644
> > > > > --- a/sysdeps/x86/cpu-tunables.c
> > > > > +++ b/sysdeps/x86/cpu-tunables.c
> > > > > @@ -223,6 +223,8 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> > > > >                                                 Fast_Unaligned_Load, 19);
> > > > >               CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> > > > >                                                 Fast_Unaligned_Copy, 19);
> > > > > +             CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> > > > > +                                               Prefer_Non_Temporal, 19);
> > > > >             }
> > > > >           break;
> > > > >         case 20:
> > > > > diff --git a/sysdeps/x86/dl-cacheinfo.h b/sysdeps/x86/dl-cacheinfo.h
> > > > > index a1c03b8903..b8ba0c098d 100644
> > > > > --- a/sysdeps/x86/dl-cacheinfo.h
> > > > > +++ b/sysdeps/x86/dl-cacheinfo.h
> > > > > @@ -992,7 +992,8 @@ dl_init_cacheinfo (struct cpu_features *cpu_features)
> > > > >       non_temporal_threshold. Enable this for both Intel and AMD hardware. */
> > > > >    unsigned long int memset_non_temporal_threshold = SIZE_MAX;
> > > > >    if (!CPU_FEATURES_ARCH_P (cpu_features, Avoid_Non_Temporal_Memset)
> > > > > -      && (cpu_features->basic.kind == arch_kind_intel
> > > > > +      && (CPU_FEATURES_ARCH_P (cpu_features, Prefer_Non_Temporal)
> > > > > +         || cpu_features->basic.kind == arch_kind_intel
> > > >
> > > > Please set Prefer_Non_Temporal for Intel and AMD.  There is no
> > > > need to check arch_kind_intel nor arch_kind_amd.
> > >
> > > We shouldn't do that. Think you a bit of a misunderstanding.
> > > Prefer_Non_Temporal will use non-temporal but *not erms*
> > > which is not what we want on intel/amd.
> > >
> > > Maybe I should rename the variable as I can see nothing about its
> > > name implies that.
> > >
> > > How about `Prefer_Non_Temporal_But_Not_ERMS` or is that too
> > > cumbersome?
> >
> > Prefer_Non_Temporal_Without_ERMS?  Please move
> > rep_stosb_threshold setting close to

Sure, thought about that but was worried is implies ERMS not being
available, but think its clearer than what we have now.
> >
> >   tunable_size = TUNABLE_GET (x86_memset_non_temporal_threshold, long
> > int, NULL);
> >   if (tunable_size > minimum_non_temporal_threshold
> >       && tunable_size <= maximum_non_temporal_threshold)
> >     memset_non_temporal_threshold = tunable_size;
> >
>
> How about something like this?

IIUC that will only skip `rep stosb` if we have ERMS. Personally not a fan
of that restrictions. We may find cases where we don't want `rep stosb` at
all despite having ERMS (as may be the case for SKX).
>
> --
> H.J.