This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] x86: Use _rdtsc intrinsic for HP_TIMING_NOW
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Tue, 2 Oct 2018 10:15:15 -0300
- Subject: Re: [PATCH] x86: Use _rdtsc intrinsic for HP_TIMING_NOW
- References: <20181001220836.20131-1-hjl.tools@gmail.com>
On 01/10/2018 19:08, H.J. Lu wrote:
> Since _rdtsc intrinsic is supported in GCC 4.9, we can use it for
> HP_TIMING_NOW.
>
> * sysdeps/i386/i686/hp-timing.h: Include <x86intrin.h>.
> (HP_TIMING_NOW): Use _rdtsc.
> * sysdeps/x86_64/hp-timing.h: Just include
> <sysdeps/i386/i686/hp-timing.h>.
> ---
> sysdeps/i386/i686/hp-timing.h | 4 +++-
> sysdeps/x86_64/hp-timing.h | 41 +----------------------------------
> 2 files changed, 4 insertions(+), 41 deletions(-)
>
> diff --git a/sysdeps/i386/i686/hp-timing.h b/sysdeps/i386/i686/hp-timing.h
> index 59af526fdb..6eee8c58be 100644
> --- a/sysdeps/i386/i686/hp-timing.h
> +++ b/sysdeps/i386/i686/hp-timing.h
> @@ -20,6 +20,8 @@
> #ifndef _HP_TIMING_H
> #define _HP_TIMING_H 1
>
> +#include <x86intrin.h>
> +
> /* We always assume having the timestamp register. */
> #define HP_TIMING_AVAIL (1)
> #define HP_SMALL_TIMING_AVAIL (1)
> @@ -35,7 +37,7 @@ typedef unsigned long long int hp_timing_t;
> running in this moment. This could be changed by using a barrier like
> 'cpuid' right before the `rdtsc' instruciton. But we are not interested
> in accurate clock cycles here so we don't do this. */
> -#define HP_TIMING_NOW(Var) __asm__ __volatile__ ("rdtsc" : "=A" (Var))
> +#define HP_TIMING_NOW(Var) ({ (Var) = _rdtsc (); })
Do we need a compound statement here? It does not use loops, switches, or
local variables.
>
> #include <hp-timing-common.h>
>
> diff --git a/sysdeps/x86_64/hp-timing.h b/sysdeps/x86_64/hp-timing.h
> index ec543bef03..46236b9777 100644
> --- a/sysdeps/x86_64/hp-timing.h
> +++ b/sysdeps/x86_64/hp-timing.h
> @@ -1,40 +1 @@
[...]
> -
> -#endif /* hp-timing.h */
> +#include <sysdeps/i386/i686/hp-timing.h>
>
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?