V2 [PATCH] x86: Use _rdtsc intrinsic for HP_TIMING_NOW

[H.J. Lu]

On Tue, Oct 2, 2018 at 11:06 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Oct 2, 2018 at 10:23 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
> >
> >
> >
> > On 02/10/2018 13:56, H.J. Lu wrote:
> > > On Tue, Oct 2, 2018 at 6:45 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >>
> > >> On Tue, Oct 2, 2018 at 6:15 AM Adhemerval Zanella
> > >> <adhemerval.zanella@linaro.org> wrote:
> > >
> > >>> I am not very found of cross abi includes like this one, wouldn't be better
> > >>> to create a 'sysdeps/x86/hp-timing.h' and for 32-bit use _rdtsc iff
> > >>> __i686__ is defined otherwise include generic header?
> > >>
> > >> I can do that.
> > >
> > > Here is the V2 patch.  OK for master?
> > >
> >
> > > NB: Checking if __i686__ isn't sufficient since __i686__ may not be
> > > defined when building for i686 class processors.
> >
> > Right, it seems gcc does not define it for -march newer than pentium4.
> >
> > > diff --git a/sysdeps/i386/i586/init-arch.h b/sysdeps/i386/i586/isa.h
> > > similarity index 89%
> > > rename from sysdeps/i386/i586/init-arch.h
> > > rename to sysdeps/i386/i586/isa.h
> > > index 72fb46c61e..a2b5d585d4 100644
> > > --- a/sysdeps/i386/i586/init-arch.h
> > > +++ b/sysdeps/i386/i586/isa.h
> > > @@ -1,4 +1,4 @@
> > > -/* Copyright (C) 2015-2018 Free Software Foundation, Inc.
> > > +/* Copyright (C) 2018 Free Software Foundation, Inc.
> > >     This file is part of the GNU C Library.
> > >
> > >     The GNU C Library is free software; you can redistribute it and/or
> > > @@ -15,5 +15,9 @@
> > >     License along with the GNU C Library; if not, see
> > >     <http://www.gnu.org/licenses/>.  */
> > >
> > > +#ifndef _isa_h
> > > +#define _isa_h
> > > +
> >
> > Not sure if this is explicit on our coding rules, but usually we
> > use uppercase caps for preprocessor guards.  I think it requires
> > a one-line description as well (same for other isa.h files).
>
> I will change that.
>
> > > -#ifndef _HP_TIMING_H
> > > -#define _HP_TIMING_H 1
> > > +#include <isa.h>
> > > +
> > > +#if MINIMUM_ISA == 686 || MINIMUM_ISA == 8664
> > > +# ifndef _HP_TIMING_H
> > > +#  define _HP_TIMING_H       1
> > > +
> >
> > The include guard inside the MINIMUM_ISA check seems off, why do
> > you need it?
>
> <sysdeps/generic/hp-timing.h> will be skipped if _HP_TIMING_H is
> defined.  That is the draw back for the single x86 hp-timing.h.

Here is the updated patch.  OK for master?

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-x86-Use-_rdtsc-intrinsic-for-HP_TIMING_NOW.patch
Type: text/x-patch
Size: 9233 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20181004/2b1678d9/attachment.bin>