Message ID | 87y49o4ntk.fsf@mid.deneb.enyo.de |
---|---|
State | Dropped |
Headers |
Received: (qmail 114854 invoked by alias); 11 Mar 2016 23:06:33 -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 114845 invoked by uid 89); 11 Mar 2016 23:06:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.6 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=386, rolandhackfrobcom, roland@hack.frob.com, 389 X-HELO: albireo.enyo.de From: Florian Weimer <fw@deneb.enyo.de> To: "H.J. Lu" <hjl.tools@gmail.com> Cc: Roland McGrath <roland@hack.frob.com>, Florian Weimer <fweimer@redhat.com>, GNU C Library <libc-alpha@sourceware.org> Subject: Re: [PATCH 1/2] Add _arch_/_cpu_ to index_*/bit_* in x86 cpu-features.h References: <1457049161-13783-1-git-send-email-hjl.tools@gmail.com> <CAMe9rOqmnA8TOOEo_KbxYnxaDDvzEcb+hqLHwK=dZ5J75W8d5Q@mail.gmail.com> <20160311214735.4CAE52C3C21@topped-with-meat.com> <CAMe9rOovPk+-DokpWoR7c6cG4bHydhaXPyNbZPyL6fRcapoD1Q@mail.gmail.com> <20160311220031.3A5672C3BC5@topped-with-meat.com> <CAMe9rOrHS0g_EZ7dgkz02SgHoxqHwp5yazt3fR9Qj9NvYG+1Jg@mail.gmail.com> <20160311222939.654342C3C24@topped-with-meat.com> <CAMe9rOo3m_x-EpY2w=ZzbLDd2MjjLhrBY6fP-ngDFGDG-Z+94A@mail.gmail.com> <874mcc636j.fsf@mid.deneb.enyo.de> <20160311225548.C6A7E2C3C24@topped-with-meat.com> <CAMe9rOoNryvq8nZ7E_00n7C19u9rX=xD9EBetwMm+3KoPNw0fQ@mail.gmail.com> Date: Sat, 12 Mar 2016 00:06:15 +0100 In-Reply-To: <CAMe9rOoNryvq8nZ7E_00n7C19u9rX=xD9EBetwMm+3KoPNw0fQ@mail.gmail.com> (H. J. Lu's message of "Fri, 11 Mar 2016 15:00:31 -0800") Message-ID: <87y49o4ntk.fsf@mid.deneb.enyo.de> MIME-Version: 1.0 Content-Type: text/plain |
Commit Message
Florian Weimer
March 11, 2016, 11:06 p.m. UTC
* H. J. Lu: > On Fri, Mar 11, 2016 at 2:55 PM, Roland McGrath <roland@hack.frob.com> wrote: >>> Roland committed a fix which included <cpu-features.h> (commit >>> 3bd80c0de2f8e7ca8020d37739339636d169957e) in tst-audit10.c I assumed >>> that this addressed this particular build failure (it does for me with >>> GCC 4.7). > > <cpu-features.h> shouldn't be used in test. Why? > #ifndef bit_AVX512F > # define bit_AVX512F > #endif This would seem more reasonable:
Comments
On Fri, Mar 11, 2016 at 3:06 PM, Florian Weimer <fw@deneb.enyo.de> wrote: > * H. J. Lu: > >> On Fri, Mar 11, 2016 at 2:55 PM, Roland McGrath <roland@hack.frob.com> wrote: >>>> Roland committed a fix which included <cpu-features.h> (commit >>>> 3bd80c0de2f8e7ca8020d37739339636d169957e) in tst-audit10.c I assumed >>>> that this addressed this particular build failure (it does for me with >>>> GCC 4.7). >> >> <cpu-features.h> shouldn't be used in test. > > Why? bit_AVX512F should come from <cpuid.h>. It is wrong to include <cpu-features.h> for bit_AVX512F.
> This would seem more reasonable:
That change is fine by me (with ChangeLog entry and verification of testing
with gcc <= 4.8, of course).
Thanks,
Roland
On 03/12/2016 12:06 AM, Florian Weimer wrote: > This would seem more reasonable: > > diff --git a/sysdeps/x86_64/tst-audit10.c b/sysdeps/x86_64/tst-audit10.c > index a487b40..0df2275 100644 > --- a/sysdeps/x86_64/tst-audit10.c > +++ b/sysdeps/x86_64/tst-audit10.c > @@ -17,13 +17,13 @@ > <http://www.gnu.org/licenses/>. */ > > #include <cpuid.h> > -#include <cpu-features.h> > > int tst_audit10_aux (void); > > static int > avx512_enabled (void) > { > +#ifdef bit_AVX512F > unsigned int eax, ebx, ecx, edx; > > if (__get_cpuid (1, &eax, &ebx, &ecx, &edx) == 0 > @@ -38,6 +38,9 @@ avx512_enabled (void) > > /* Verify that ZMM, YMM and XMM states are enabled. */ > return (eax & 0xe6) == 0xe6; > +#else > + return 0; > +#endif > } > > static int I have committed this, after testing compilation with GCC 4.7 and GCC 5.3. Bug 19860 mentions a second compilation error, which I did not see. I'm waiting for details on that one, maybe it was a spurious issue. Florian
> > --- a/sysdeps/x86_64/tst-audit10.c > > +++ b/sysdeps/x86_64/tst-audit10.c > > @@ -17,13 +17,13 @@ > > <http://www.gnu.org/licenses/>. */ > > > > #include <cpuid.h> > > -#include <cpu-features.h> > > > > int tst_audit10_aux (void); > > > > static int > > avx512_enabled (void) > > { > > +#ifdef bit_AVX512F > > unsigned int eax, ebx, ecx, edx; > > > > if (__get_cpuid (1, &eax, &ebx, &ecx, &edx) == 0 > > @@ -38,6 +38,9 @@ avx512_enabled (void) > > > > /* Verify that ZMM, YMM and XMM states are enabled. */ > > return (eax & 0xe6) == 0xe6; > > +#else > > + return 0; > > +#endif > > } > > > > static int > > I have committed this, after testing compilation with GCC 4.7 and GCC > 5.3. Bug 19860 mentions a second compilation error, which I did not > see. I'm waiting for details on that one, maybe it was a spurious issue. It occurred to me later that having this use #ifdef HAVE_AVX512_SUPPORT would be better. That is, we expect a compiler that accepts -mavx512f to provide a <cpuid.h> that defines bit_AVX512F. If we have -mavx512f support but somehow get a <cpuid.h> missing bit_AVX512F, that should be a build error because the compiler installation is self-inconsistent. Thanks, Roland
diff --git a/sysdeps/x86_64/tst-audit10.c b/sysdeps/x86_64/tst-audit10.c index a487b40..0df2275 100644 --- a/sysdeps/x86_64/tst-audit10.c +++ b/sysdeps/x86_64/tst-audit10.c @@ -17,13 +17,13 @@ <http://www.gnu.org/licenses/>. */ #include <cpuid.h> -#include <cpu-features.h> int tst_audit10_aux (void); static int avx512_enabled (void) { +#ifdef bit_AVX512F unsigned int eax, ebx, ecx, edx; if (__get_cpuid (1, &eax, &ebx, &ecx, &edx) == 0 @@ -38,6 +38,9 @@ avx512_enabled (void) /* Verify that ZMM, YMM and XMM states are enabled. */ return (eax & 0xe6) == 0xe6; +#else + return 0; +#endif } static int