* Re: [PATCH] x86-64: Simplify minimum ISA check ifdef conditional with if [not found] ` <CAMe9rOr6tLZ5Na8XN1QMa4zO2kNw1+cW2E9g=-wat+tQVNqNAA@mail.gmail.com> @ 2024-04-13 15:59 ` H.J. Lu 2024-04-13 17:52 ` Florian Weimer 0 siblings, 1 reply; 4+ messages in thread From: H.J. Lu @ 2024-04-13 15:59 UTC (permalink / raw) To: Sunil K Pandey, Florian Weimer; +Cc: Libc-stable Mailing List On Sun, Mar 3, 2024 at 1:46 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Fri, Mar 1, 2024 at 4:40 PM Sunil K Pandey <skpgkp2@gmail.com> wrote: > > > > Replace minimum ISA check ifdef conditional with if. Since > > MINIMUM_X86_ISA_LEVEL and AVX_X86_ISA_LEVEL are compile time constants, > > compiler will perform constant folding optimization getting same result. > > --- > > sysdeps/x86/cpu-features.c | 19 ++++++++----------- > > 1 file changed, 8 insertions(+), 11 deletions(-) > > > > diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c > > index e7c7ece462..4ea373dffa 100644 > > --- a/sysdeps/x86/cpu-features.c > > +++ b/sysdeps/x86/cpu-features.c > > @@ -1196,9 +1196,8 @@ no_cpuid: > > TUNABLE_CALLBACK (set_x86_shstk)); > > #endif > > > > -#if MINIMUM_X86_ISA_LEVEL < AVX_X86_ISA_LEVEL > > - if (GLRO(dl_x86_cpu_features).xsave_state_size != 0) > > -#endif > > + if (MINIMUM_X86_ISA_LEVEL >= AVX_X86_ISA_LEVEL > > + || (GLRO(dl_x86_cpu_features).xsave_state_size != 0)) > > { > > if (CPU_FEATURE_USABLE_P (cpu_features, XSAVEC)) > > { > > @@ -1219,24 +1218,22 @@ no_cpuid: > > #endif > > } > > } > > -#if MINIMUM_X86_ISA_LEVEL < AVX_X86_ISA_LEVEL > > else > > { > > -# ifdef __x86_64__ > > +#ifdef __x86_64__ > > GLRO(dl_x86_64_runtime_resolve) = _dl_runtime_resolve_fxsave; > > -# ifdef SHARED > > +# ifdef SHARED > > GLRO(dl_x86_tlsdesc_dynamic) = _dl_tlsdesc_dynamic_fxsave; > > -# endif > > -# else > > -# ifdef SHARED > > +# endif > > +#else > > +# ifdef SHARED > > if (CPU_FEATURE_USABLE_P (cpu_features, FXSR)) > > GLRO(dl_x86_tlsdesc_dynamic) = _dl_tlsdesc_dynamic_fxsave; > > else > > GLRO(dl_x86_tlsdesc_dynamic) = _dl_tlsdesc_dynamic_fnsave; > > -# endif > > # endif > > - } > > #endif > > + } > > > > #ifdef SHARED > > # ifdef __x86_64__ > > -- > > 2.43.0 > > > > LGTM. > > Reviewed-by: H.J. Lu <hjl.tools@gmail.com> > > Thanks. > > -- > H.J. Should it also be backported to glibc 2.39 together with commit befe2d3c4dec8be2cdd01a47132e47bdb7020922 Author: H.J. Lu <hjl.tools@gmail.com> Date: Wed Feb 28 09:51:14 2024 -0800 x86-64: Don't use SSE resolvers for ISA level 3 or above -- H.J. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86-64: Simplify minimum ISA check ifdef conditional with if 2024-04-13 15:59 ` [PATCH] x86-64: Simplify minimum ISA check ifdef conditional with if H.J. Lu @ 2024-04-13 17:52 ` Florian Weimer 2024-04-13 18:04 ` H.J. Lu 0 siblings, 1 reply; 4+ messages in thread From: Florian Weimer @ 2024-04-13 17:52 UTC (permalink / raw) To: H.J. Lu; +Cc: Sunil K Pandey, Libc-stable Mailing List * H. J. Lu: > Should it also be backported to glibc 2.39 together with > > commit befe2d3c4dec8be2cdd01a47132e47bdb7020922 > Author: H.J. Lu <hjl.tools@gmail.com> > Date: Wed Feb 28 09:51:14 2024 -0800 > > x86-64: Don't use SSE resolvers for ISA level 3 or above This is the one that breaks valgrind, isn't it? I think we'll do downstream backports of some of the x86-64 optimizations, but in the past, there was pushback against doing those upstream. Thanks, Florian ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86-64: Simplify minimum ISA check ifdef conditional with if 2024-04-13 17:52 ` Florian Weimer @ 2024-04-13 18:04 ` H.J. Lu 2024-04-13 18:22 ` Florian Weimer 0 siblings, 1 reply; 4+ messages in thread From: H.J. Lu @ 2024-04-13 18:04 UTC (permalink / raw) To: Florian Weimer; +Cc: Sunil K Pandey, Libc-stable Mailing List On Sat, Apr 13, 2024 at 10:52 AM Florian Weimer <fweimer@redhat.com> wrote: > > * H. J. Lu: > > > Should it also be backported to glibc 2.39 together with > > > > commit befe2d3c4dec8be2cdd01a47132e47bdb7020922 > > Author: H.J. Lu <hjl.tools@gmail.com> > > Date: Wed Feb 28 09:51:14 2024 -0800 > > > > x86-64: Don't use SSE resolvers for ISA level 3 or above > > This is the one that breaks valgrind, isn't it? No. -march=x86-64-v3 triggers the valgrind error in flibc test. This commit fixes the -march=x86-64-v3 build. commit b6e3898194bbae78910bbe9cd086937014961e45 Author: Sunil K Pandey <skpgkp2@gmail.com> Date: Thu Feb 29 17:57:02 2024 -0800 x86-64: Simplify minimum ISA check ifdef conditional with if updates this commit. Backporting the above commit makes the backported code look similar to upstream. > I think we'll do downstream backports of some of the x86-64 > optimizations, but in the past, there was pushback against doing those > upstream. > > Thanks, > Florian > -- H.J. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86-64: Simplify minimum ISA check ifdef conditional with if 2024-04-13 18:04 ` H.J. Lu @ 2024-04-13 18:22 ` Florian Weimer 0 siblings, 0 replies; 4+ messages in thread From: Florian Weimer @ 2024-04-13 18:22 UTC (permalink / raw) To: H.J. Lu; +Cc: Sunil K Pandey, Libc-stable Mailing List * H. J. Lu: > On Sat, Apr 13, 2024 at 10:52 AM Florian Weimer <fweimer@redhat.com> wrote: >> >> * H. J. Lu: >> >> > Should it also be backported to glibc 2.39 together with >> > >> > commit befe2d3c4dec8be2cdd01a47132e47bdb7020922 >> > Author: H.J. Lu <hjl.tools@gmail.com> >> > Date: Wed Feb 28 09:51:14 2024 -0800 >> > >> > x86-64: Don't use SSE resolvers for ISA level 3 or above >> >> This is the one that breaks valgrind, isn't it? > > No. -march=x86-64-v3 triggers the valgrind error in flibc test. This > commit fixes the -march=x86-64-v3 build. > > commit b6e3898194bbae78910bbe9cd086937014961e45 > Author: Sunil K Pandey <skpgkp2@gmail.com> > Date: Thu Feb 29 17:57:02 2024 -0800 > > x86-64: Simplify minimum ISA check ifdef conditional with if > > updates this commit. Backporting the above commit makes the > backported code look similar to upstream. yes, I was confused, after all I backported the “x86-64: Don't use SSE resolvers for ISA level 3 or above” commit and saw the valgrind failure with it! I'll run my tests again for the additional backport and push it. Thanks, Florian ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-13 18:22 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CAMAf5_fVOYmBsTsfNHNQOBXjLYRF=NQ2gAduJMFwkPOMcW4yCw@mail.gmail.com> [not found] ` <20240302003843.3333713-1-skpgkp2@gmail.com> [not found] ` <CAMe9rOr6tLZ5Na8XN1QMa4zO2kNw1+cW2E9g=-wat+tQVNqNAA@mail.gmail.com> 2024-04-13 15:59 ` [PATCH] x86-64: Simplify minimum ISA check ifdef conditional with if H.J. Lu 2024-04-13 17:52 ` Florian Weimer 2024-04-13 18:04 ` H.J. Lu 2024-04-13 18:22 ` Florian Weimer
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).