[PATCH] PowerPC - Add a faster way to read the Time Base register

[Ryan S. Arnold]

On Tue, Feb 14, 2012 at 2:03 PM, Richard Henderson <rth@twiddle.net> wrote:
> On 02/14/2012 07:30 AM, Tulio Magno Quites Machado Filho wrote:
>> Add macro __PPC_GETTIMEBASE() to directly read the Time Base register.
>> This is required for applications that measure time at high frequencies
>> with high precision that can't afford a syscall.
>>
>> My FSF copyright assignment is on file.
>>
>> 2012-02-06  Tulio Magno Quites Machado Filho  <tuliom@linux.vnet.ibm.com>
>>
>>       * sysdeps/unix/sysv/linux/powerpc/sys/user.h: New macro definition.
>>       * sysdeps/unix/sysv/linux/powerpc/Makefile (tests): Add
>>       test-gettimebase.
>>       * sysdeps/unix/sysv/linux/powerpc/test-gettimebase.c: Test for
>>       __PPC_GETTIMEBASE() to catch future ISA opcode/insn changes.
>> ---
>>  sysdeps/unix/sysv/linux/powerpc/Makefile           |    2 +
>>  sysdeps/unix/sysv/linux/powerpc/sys/user.h         |   40 +++++++++++++++++++-
>>  sysdeps/unix/sysv/linux/powerpc/test-gettimebase.c |   35 +++++++++++++++++
>>  3 files changed, 76 insertions(+), 1 deletions(-)
>>  create mode 100644 sysdeps/unix/sysv/linux/powerpc/test-gettimebase.c
>>
>> diff --git a/sysdeps/unix/sysv/linux/powerpc/Makefile b/sysdeps/unix/sysv/linux/powerpc/Makefile
>> index 55311a4..a2ab09a 100644
>> --- a/sysdeps/unix/sysv/linux/powerpc/Makefile
>> +++ b/sysdeps/unix/sysv/linux/powerpc/Makefile
>> @@ -15,3 +15,5 @@ endif
>>  ifeq ($(subdir),elf)
>>  sysdep_routines += dl-vdso
>>  endif
>> +
>> +tests += test-gettimebase
>> diff --git a/sysdeps/unix/sysv/linux/powerpc/sys/user.h b/sysdeps/unix/sysv/linux/powerpc/sys/user.h
>> index 5fa3745..301e64a 100644
>> --- a/sysdeps/unix/sysv/linux/powerpc/sys/user.h
>> +++ b/sysdeps/unix/sysv/linux/powerpc/sys/user.h
>> @@ -1,4 +1,4 @@
>> -/* Copyright (C) 1998 Free Software Foundation, Inc.
>> +/* Copyright (C) 1998, 2012 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
>> @@ -36,4 +36,42 @@ struct user {
>>       char            u_comm[32];             /* user command name */
>>  };
>>
>> +typedef unsigned long long int __ppc_timebase;
>> +
>> +#define __STRINGIFY(...) #__VA_ARGS__
>> +#define STRINGIFY(...) __STRINGIFY(__VA_ARGS__)
>> +
>> +#define SPRN_TBRL 268 /* Time Base Read Lower Register */
>> +
>> +#ifdef __powerpc64__
>> +# define __PPC_GETTIMEBASE()                                 \
>> +  ({                                                         \
>> +    __ppc_timebase __tb;                                     \
>> +    __asm__ volatile (                                               \
>> +                   "mfspr %[tb], " STRINGIFY(SPRN_TBRL) "\n" \
>
> Is there a good reason why you're stringify-ing this constant
> rather than just writing "mfspr %0, 268"?  You certainly don't
> want to be polluting the namespace with an unadorned "STRINGIFY".

We didn't want a naked (non-descriptive) opcode parameter.  There must
be a way to do this that takes the definition as a variable, like
perhaps the following (totally untested) code:

    __asm__ volatile (                                         \
                     "mfspr %[tb], %2\n" \
                     : [tb]"=r" (__tb)                         \
                     : "i" (SPRN_TBRL)  );


> Is there a good reason this is a macro rather than an inline?

I don't see why this couldn't be a static inline.

Ryan S. Arnold