Message ID | 20150817231005.GA24205@intel.com |
---|---|
State | Superseded |
Headers |
Received: (qmail 54500 invoked by alias); 17 Aug 2015 23:10:16 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 54472 invoked by uid 89); 17 Aug 2015 23:10:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, NO_DNS_FOR_FROM, RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mga14.intel.com X-ExtLoop1: 1 Date: Mon, 17 Aug 2015 16:10:05 -0700 From: "H.J. Lu" <hongjiu.lu@intel.com> To: GNU C Library <libc-alpha@sourceware.org> Subject: [PATCH] Define HAS_CPUID/HAS_I586/HAS_I686 from -march= Message-ID: <20150817231005.GA24205@intel.com> Reply-To: "H.J. Lu" <hjl.tools@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) |
Commit Message
Lu, Hongjiu
Aug. 17, 2015, 11:10 p.m. UTC
cpuid, i586 and i686 instructions are available if the processor specified by -march= supports them. We can use this information to determine whether those instructions can be used safely. OK for master? H.J. --- * sysdeps/x86/cpu-features.c (init_cpu_features): Check whether cpuid is available only if HAS_CPUID is 0. * sysdeps/x86/cpu-features.h (HAS_CPUID): New. (HAS_I586): Likewise. (HAS_I686): Likewise. --- sysdeps/x86/cpu-features.c | 4 ++-- sysdeps/x86/cpu-features.h | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-)
Comments
On Mon, Aug 17, 2015 at 04:10:05PM -0700, H.J. Lu wrote: > cpuid, i586 and i686 instructions are available if the processor > specified by -march= supports them. We can use this information > to determine whether those instructions can be used safely. > > OK for master? > Patch itself looks ok. It fixes theoretical problem that if there would be new processor without cpuid it would cause problem. But could you explain rationale? Here you replaced one like with equivalent longer macro and HAS_I586 is never used. Do you plan followup patches that use that?
On Mon, Aug 17, 2015 at 10:24 PM, Ondřej Bílka <neleai@seznam.cz> wrote: > On Mon, Aug 17, 2015 at 04:10:05PM -0700, H.J. Lu wrote: >> cpuid, i586 and i686 instructions are available if the processor >> specified by -march= supports them. We can use this information >> to determine whether those instructions can be used safely. >> >> OK for master? >> > Patch itself looks ok. It fixes theoretical problem that if there would > be new processor without cpuid it would cause problem. > > But could you explain rationale? Here you replaced one like with > equivalent longer macro and HAS_I586 is never used. Do you plan followup > patches that use that? I will use HAS_I586 and HAS_I686 in multi-arch for i486 and i586 I am working on.
On 17 Aug 2015 16:10, H.J. Lu wrote: > +#ifdef __x86_64__ > +# define HAS_CPUID 1 > +#elif defined __pentium__ > +# define HAS_CPUID 1 > +# define HAS_I586 1 > +# define HAS_I686 0 > +#elif (defined __pentiumpro__ || defined __pentium4__ \ > + || defined __nocona__ || defined __atom__ \ > + || defined __core2__ || defined __corei7__ \ > + || defined __corei7_avx__ || defined __core_avx2__ \ > + || defined __nehalem__ || defined __sandybridge__ \ > + || defined __haswell__ || defined __knl__ \ > + || defined __bonnell__ || defined __silvermont__ \ > + || defined __k6__ || defined __k8__ \ > + || defined __athlon__ || defined __amdfam10__ \ > + || defined __bdver1__ || defined __bdver2__ \ > + || defined __bdver3__ || defined __bdver4__ \ > + || defined __btver1__ || defined __btver2__) > +# define HAS_CPUID 1 > +# define HAS_I586 1 > +# define HAS_I686 1 > +#else > +# define HAS_CPUID 0 > +# define HAS_I586 0 > +# define HAS_I686 0 > +#endif why is testing for __i686__ & __i586__ unacceptable ? -mike
On Tue, Aug 18, 2015 at 7:32 AM, Mike Frysinger <vapier@gentoo.org> wrote: > On 17 Aug 2015 16:10, H.J. Lu wrote: >> +#ifdef __x86_64__ >> +# define HAS_CPUID 1 >> +#elif defined __pentium__ >> +# define HAS_CPUID 1 >> +# define HAS_I586 1 >> +# define HAS_I686 0 >> +#elif (defined __pentiumpro__ || defined __pentium4__ \ >> + || defined __nocona__ || defined __atom__ \ >> + || defined __core2__ || defined __corei7__ \ >> + || defined __corei7_avx__ || defined __core_avx2__ \ >> + || defined __nehalem__ || defined __sandybridge__ \ >> + || defined __haswell__ || defined __knl__ \ >> + || defined __bonnell__ || defined __silvermont__ \ >> + || defined __k6__ || defined __k8__ \ >> + || defined __athlon__ || defined __amdfam10__ \ >> + || defined __bdver1__ || defined __bdver2__ \ >> + || defined __bdver3__ || defined __bdver4__ \ >> + || defined __btver1__ || defined __btver2__) >> +# define HAS_CPUID 1 >> +# define HAS_I586 1 >> +# define HAS_I686 1 >> +#else >> +# define HAS_CPUID 0 >> +# define HAS_I586 0 >> +# define HAS_I686 0 >> +#endif > > why is testing for __i686__ & __i586__ unacceptable ? > -mike -march=haswell doesn't define __i686__ since __i686__ is mapped to -mach=i686.
On 08/18/2015 09:34 AM, H.J. Lu wrote: > On Mon, Aug 17, 2015 at 10:24 PM, Ondřej Bílka <neleai@seznam.cz> wrote: >> On Mon, Aug 17, 2015 at 04:10:05PM -0700, H.J. Lu wrote: >>> cpuid, i586 and i686 instructions are available if the processor >>> specified by -march= supports them. We can use this information >>> to determine whether those instructions can be used safely. >>> >>> OK for master? >>> >> Patch itself looks ok. It fixes theoretical problem that if there would >> be new processor without cpuid it would cause problem. >> >> But could you explain rationale? Here you replaced one like with >> equivalent longer macro and HAS_I586 is never used. Do you plan followup >> patches that use that? > > I will use HAS_I586 and HAS_I686 in multi-arch for i486 and i586 > I am working on. Does this include folding i486 up a directory and doing away with the i386 support? Since we need the i486 instructions to implement NPTL. https://sourceware.org/glibc/wiki/Development_Todo/Master#i386 Cheers, Carlos.
On Tue, Aug 18, 2015 at 8:49 AM, Carlos O'Donell <carlos@redhat.com> wrote: > On 08/18/2015 09:34 AM, H.J. Lu wrote: >> On Mon, Aug 17, 2015 at 10:24 PM, Ondřej Bílka <neleai@seznam.cz> wrote: >>> On Mon, Aug 17, 2015 at 04:10:05PM -0700, H.J. Lu wrote: >>>> cpuid, i586 and i686 instructions are available if the processor >>>> specified by -march= supports them. We can use this information >>>> to determine whether those instructions can be used safely. >>>> >>>> OK for master? >>>> >>> Patch itself looks ok. It fixes theoretical problem that if there would >>> be new processor without cpuid it would cause problem. >>> >>> But could you explain rationale? Here you replaced one like with >>> equivalent longer macro and HAS_I586 is never used. Do you plan followup >>> patches that use that? >> >> I will use HAS_I586 and HAS_I686 in multi-arch for i486 and i586 >> I am working on. > > Does this include folding i486 up a directory and doing away with the > i386 support? Since we need the i486 instructions to implement NPTL. > > https://sourceware.org/glibc/wiki/Development_Todo/Master#i386 > I will investigate it and it will take a while.
On 08/18/2015 11:52 AM, H.J. Lu wrote: > On Tue, Aug 18, 2015 at 8:49 AM, Carlos O'Donell <carlos@redhat.com> wrote: >> On 08/18/2015 09:34 AM, H.J. Lu wrote: >>> On Mon, Aug 17, 2015 at 10:24 PM, Ondřej Bílka <neleai@seznam.cz> wrote: >>>> On Mon, Aug 17, 2015 at 04:10:05PM -0700, H.J. Lu wrote: >>>>> cpuid, i586 and i686 instructions are available if the processor >>>>> specified by -march= supports them. We can use this information >>>>> to determine whether those instructions can be used safely. >>>>> >>>>> OK for master? >>>>> >>>> Patch itself looks ok. It fixes theoretical problem that if there would >>>> be new processor without cpuid it would cause problem. >>>> >>>> But could you explain rationale? Here you replaced one like with >>>> equivalent longer macro and HAS_I586 is never used. Do you plan followup >>>> patches that use that? >>> >>> I will use HAS_I586 and HAS_I686 in multi-arch for i486 and i586 >>> I am working on. >> >> Does this include folding i486 up a directory and doing away with the >> i386 support? Since we need the i486 instructions to implement NPTL. >> >> https://sourceware.org/glibc/wiki/Development_Todo/Master#i386 >> > > I will investigate it and it will take a while. > Thanks :-) Cheers, Carlos.
On 18 Aug 2015 07:56, H.J. Lu wrote: > On Tue, Aug 18, 2015 at 7:32 AM, Mike Frysinger wrote: > > On 17 Aug 2015 16:10, H.J. Lu wrote: > >> +#ifdef __x86_64__ > >> +# define HAS_CPUID 1 > >> +#elif defined __pentium__ > >> +# define HAS_CPUID 1 > >> +# define HAS_I586 1 > >> +# define HAS_I686 0 > >> +#elif (defined __pentiumpro__ || defined __pentium4__ \ > >> + || defined __nocona__ || defined __atom__ \ > >> + || defined __core2__ || defined __corei7__ \ > >> + || defined __corei7_avx__ || defined __core_avx2__ \ > >> + || defined __nehalem__ || defined __sandybridge__ \ > >> + || defined __haswell__ || defined __knl__ \ > >> + || defined __bonnell__ || defined __silvermont__ \ > >> + || defined __k6__ || defined __k8__ \ > >> + || defined __athlon__ || defined __amdfam10__ \ > >> + || defined __bdver1__ || defined __bdver2__ \ > >> + || defined __bdver3__ || defined __bdver4__ \ > >> + || defined __btver1__ || defined __btver2__) > >> +# define HAS_CPUID 1 > >> +# define HAS_I586 1 > >> +# define HAS_I686 1 > >> +#else > >> +# define HAS_CPUID 0 > >> +# define HAS_I586 0 > >> +# define HAS_I686 0 > >> +#endif > > > > why is testing for __i686__ & __i586__ unacceptable ? > > -march=haswell doesn't define __i686__ since __i686__ is > mapped to -mach=i686. that's not entirely accurate. i did check before posting, but it looks like the first two entries do set up i586 and i686 correctly (and i stopped after spot checking the first two). so i guess change __pentium__ to __i586__ and __pentiumpro__ to __i686__ ? -mike
On Tue, Aug 18, 2015 at 9:02 AM, Mike Frysinger <vapier@gentoo.org> wrote: > On 18 Aug 2015 07:56, H.J. Lu wrote: >> On Tue, Aug 18, 2015 at 7:32 AM, Mike Frysinger wrote: >> > On 17 Aug 2015 16:10, H.J. Lu wrote: >> >> +#ifdef __x86_64__ >> >> +# define HAS_CPUID 1 >> >> +#elif defined __pentium__ >> >> +# define HAS_CPUID 1 >> >> +# define HAS_I586 1 >> >> +# define HAS_I686 0 >> >> +#elif (defined __pentiumpro__ || defined __pentium4__ \ >> >> + || defined __nocona__ || defined __atom__ \ >> >> + || defined __core2__ || defined __corei7__ \ >> >> + || defined __corei7_avx__ || defined __core_avx2__ \ >> >> + || defined __nehalem__ || defined __sandybridge__ \ >> >> + || defined __haswell__ || defined __knl__ \ >> >> + || defined __bonnell__ || defined __silvermont__ \ >> >> + || defined __k6__ || defined __k8__ \ >> >> + || defined __athlon__ || defined __amdfam10__ \ >> >> + || defined __bdver1__ || defined __bdver2__ \ >> >> + || defined __bdver3__ || defined __bdver4__ \ >> >> + || defined __btver1__ || defined __btver2__) >> >> +# define HAS_CPUID 1 >> >> +# define HAS_I586 1 >> >> +# define HAS_I686 1 >> >> +#else >> >> +# define HAS_CPUID 0 >> >> +# define HAS_I586 0 >> >> +# define HAS_I686 0 >> >> +#endif >> > >> > why is testing for __i686__ & __i586__ unacceptable ? >> >> -march=haswell doesn't define __i686__ since __i686__ is >> mapped to -mach=i686. > > that's not entirely accurate. i did check before posting, but it looks like > the first two entries do set up i586 and i686 correctly (and i stopped after > spot checking the first two). so i guess change __pentium__ to __i586__ and > __pentiumpro__ to __i686__ ? > -mike My change is based on sysdeps/x86/bits/string.h and sysdeps/x86/bits/byteswap.h. None of them check __i586__ nor __i686__.
On Tue, 18 Aug 2015, H.J. Lu wrote: > My change is based on sysdeps/x86/bits/string.h and > sysdeps/x86/bits/byteswap.h. None of them check > __i586__ nor __i686__. Maybe (a) there should be a new (installed) bits/ header for x86, say bits/x86-arch.h, that defines __x86_arch to 4, 5 or 6 depending on such macros, so the logic doesn't need duplicating; (b) the header should test for the probably much smaller number of 486 / 586 processors supported by GCC, with the default being i686, to avoid keeping needing updating it for future processors; (c) GCC should add such a predefined macro, with bits/x86-arch.h only defining it itself if not already defined, as further future-proofing against anyone adding new support for an i486-class or i586-class processor with new predefined macros? (Cf. sysdeps/arm/memcpy.S working based on a list of ARM architectures without preload support, instead of a list with it; and the __ARM_ARCH macro from ACLE in newer versions of GCC.)
On Tue, Aug 18, 2015 at 9:16 AM, Joseph Myers <joseph@codesourcery.com> wrote: > On Tue, 18 Aug 2015, H.J. Lu wrote: > >> My change is based on sysdeps/x86/bits/string.h and >> sysdeps/x86/bits/byteswap.h. None of them check >> __i586__ nor __i686__. > > Maybe > > (a) there should be a new (installed) bits/ header for x86, say > bits/x86-arch.h, that defines __x86_arch to 4, 5 or 6 depending on such > macros, so the logic doesn't need duplicating; > We can add cpu-features.h to installed header to provide both run-time and compile-time CPU feature detection.
On Tue, 18 Aug 2015, H.J. Lu wrote: > On Tue, Aug 18, 2015 at 9:16 AM, Joseph Myers <joseph@codesourcery.com> wrote: > > On Tue, 18 Aug 2015, H.J. Lu wrote: > > > >> My change is based on sysdeps/x86/bits/string.h and > >> sysdeps/x86/bits/byteswap.h. None of them check > >> __i586__ nor __i686__. > > > > Maybe > > > > (a) there should be a new (installed) bits/ header for x86, say > > bits/x86-arch.h, that defines __x86_arch to 4, 5 or 6 depending on such > > macros, so the logic doesn't need duplicating; > > > > We can add cpu-features.h to installed header to provide both > run-time and compile-time CPU feature detection. What I'm suggesting requires a header that does not define or use any identifiers outside of the implementation namespace, so that it can be included by other installed headers. This is substantially different from cpu-features.h.
On 18 Aug 2015 09:08, H.J. Lu wrote: > On Tue, Aug 18, 2015 at 9:02 AM, Mike Frysinger wrote: > > On 18 Aug 2015 07:56, H.J. Lu wrote: > >> On Tue, Aug 18, 2015 at 7:32 AM, Mike Frysinger wrote: > >> > On 17 Aug 2015 16:10, H.J. Lu wrote: > >> >> +#ifdef __x86_64__ > >> >> +# define HAS_CPUID 1 > >> >> +#elif defined __pentium__ > >> >> +# define HAS_CPUID 1 > >> >> +# define HAS_I586 1 > >> >> +# define HAS_I686 0 > >> >> +#elif (defined __pentiumpro__ || defined __pentium4__ \ > >> >> + || defined __nocona__ || defined __atom__ \ > >> >> + || defined __core2__ || defined __corei7__ \ > >> >> + || defined __corei7_avx__ || defined __core_avx2__ \ > >> >> + || defined __nehalem__ || defined __sandybridge__ \ > >> >> + || defined __haswell__ || defined __knl__ \ > >> >> + || defined __bonnell__ || defined __silvermont__ \ > >> >> + || defined __k6__ || defined __k8__ \ > >> >> + || defined __athlon__ || defined __amdfam10__ \ > >> >> + || defined __bdver1__ || defined __bdver2__ \ > >> >> + || defined __bdver3__ || defined __bdver4__ \ > >> >> + || defined __btver1__ || defined __btver2__) > >> >> +# define HAS_CPUID 1 > >> >> +# define HAS_I586 1 > >> >> +# define HAS_I686 1 > >> >> +#else > >> >> +# define HAS_CPUID 0 > >> >> +# define HAS_I586 0 > >> >> +# define HAS_I686 0 > >> >> +#endif > >> > > >> > why is testing for __i686__ & __i586__ unacceptable ? > >> > >> -march=haswell doesn't define __i686__ since __i686__ is > >> mapped to -mach=i686. > > > > that's not entirely accurate. i did check before posting, but it looks like > > the first two entries do set up i586 and i686 correctly (and i stopped after > > spot checking the first two). so i guess change __pentium__ to __i586__ and > > __pentiumpro__ to __i686__ ? > > My change is based on sysdeps/x86/bits/string.h and > sysdeps/x86/bits/byteswap.h. None of them check > __i586__ nor __i686__. i'm aware of the provenance. that's not a reason to not improve things :). -mike
diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c index 2d744ac..16d78c5 100644 --- a/sysdeps/x86/cpu-features.c +++ b/sysdeps/x86/cpu-features.c @@ -40,7 +40,7 @@ init_cpu_features (struct cpu_features *cpu_features) unsigned int model = 0; enum cpu_features_kind kind; -#if !defined __i586__ && !defined __i686__ && !defined __x86_64__ +#if !HAS_CPUID if (__get_cpuid_max (0, 0) == 0) { kind = arch_kind_other; @@ -212,7 +212,7 @@ init_cpu_features (struct cpu_features *cpu_features) } } -#if !defined __i586__ && !defined __i686__ && !defined __x86_64__ +#if !HAS_CPUID no_cpuid: #endif diff --git a/sysdeps/x86/cpu-features.h b/sysdeps/x86/cpu-features.h index fce30c3..795d653 100644 --- a/sysdeps/x86/cpu-features.h +++ b/sysdeps/x86/cpu-features.h @@ -251,4 +251,31 @@ extern const struct cpu_features *__get_cpu_features (void) #endif /* !__ASSEMBLER__ */ +#ifdef __x86_64__ +# define HAS_CPUID 1 +#elif defined __pentium__ +# define HAS_CPUID 1 +# define HAS_I586 1 +# define HAS_I686 0 +#elif (defined __pentiumpro__ || defined __pentium4__ \ + || defined __nocona__ || defined __atom__ \ + || defined __core2__ || defined __corei7__ \ + || defined __corei7_avx__ || defined __core_avx2__ \ + || defined __nehalem__ || defined __sandybridge__ \ + || defined __haswell__ || defined __knl__ \ + || defined __bonnell__ || defined __silvermont__ \ + || defined __k6__ || defined __k8__ \ + || defined __athlon__ || defined __amdfam10__ \ + || defined __bdver1__ || defined __bdver2__ \ + || defined __bdver3__ || defined __bdver4__ \ + || defined __btver1__ || defined __btver2__) +# define HAS_CPUID 1 +# define HAS_I586 1 +# define HAS_I686 1 +#else +# define HAS_CPUID 0 +# define HAS_I586 0 +# define HAS_I686 0 +#endif + #endif /* cpu_features_h */