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

[H.J. Lu]

On Tue, Aug 13, 2024 at 6:35 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> On Tue, Aug 13, 2024 at 9:29 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Aug 13, 2024 at 6:25 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> > >
> > > 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
> >
> > The code is
> >
> > +  else if (CPU_FEATURES_ARCH_P (cpu_features, Prefer_Non_Temporal)
> > +    && !CPU_FEATURE_USABLE_P (cpu_features, ERMS))
> > +    rep_stosb_threshold = tunable_size;
> >
> > It will skip `rep stosb` if we DON'T have ERMS.
>
> Yes but it will only set the `rep stosb` threshold if we don't have ERMS
> so req non-erms to use the `rep stosb` codes as staging point for non-temporal
> which is the point of this patch.
>
> >
> > > 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).
> >
> > We can make ERMS unsable in this case.
>
> That would work, although I don't think its easier to start juggling
> capabilities.
> I don't think we should start disabling the usability of features to
> meet preferences.

Each feature has a bit to indicate it is available, another bit to
indicate it is usable.  If a feature should be avoided, we turn
off the usable bit.  This is how it works.

> >
> > > >
> > > > --
> > > > H.J.
> >
> >
> >
> > --
> > H.J.



-- 
H.J.