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

["Ryan S. Arnold" <ryan dot arnold at gmail dot com>]

On Tue, Apr 17, 2012 at 3:22 PM, Tulio Magno Quites Machado Filho
<tuliom@linux.vnet.ibm.com> wrote:
> Add function __ppc_get_timebase() 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.
>
> 2012-04-09 Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
>
> Â Â Â Â[BZ #13743]
> Â Â Â Â* sysdeps/powerpc/sys/platform/ppc.h: New file for PowerPC features.
> Â Â Â Â* sysdeps/powerpc/Makefile (tests): Add test-gettimebase.
> Â Â Â Â(sysdep_headers): Include sys/platform/ppc.h.
> Â Â Â Â* sysdeps/powerpc/test-gettimebase.c: Test for
> Â Â Â Â__ppc_get_timebase() to catch future ISA opcode/insn changes.
> Â Â Â Â* manual/Makefile (appendices): Include platform.texi.
> Â Â Â Â* manual/contrib.texi: Likewise.
> Â Â Â Â* manual/maint.texi: Document how to include platform-specific
> Â Â Â Âheaders.
> Â Â Â Â* manual/platform.texi: New file. Document the new features.
> ---
> Âmanual/Makefile          Â|  Â3 +-
> Âmanual/contrib.texi        Â|  Â2 +-
> Âmanual/maint.texi         Â|  76 +++++++++++++++++++++++++++++++++++-
> Âmanual/platform.texi        |  28 +++++++++++++
> Âsysdeps/powerpc/Makefile      |  Â4 ++
> Âsysdeps/powerpc/sys/platform/ppc.h | Â 53 +++++++++++++++++++++++++
> Âsysdeps/powerpc/test-gettimebase.c | Â 46 ++++++++++++++++++++++
> Â7 files changed, 209 insertions(+), 3 deletions(-)
> Âcreate mode 100644 manual/platform.texi
> Âcreate mode 100644 sysdeps/powerpc/sys/platform/ppc.h
> Âcreate mode 100644 sysdeps/powerpc/test-gettimebase.c
>
> diff --git a/manual/Makefile b/manual/Makefile
> index daddd29..2dd7c96 100644
> --- a/manual/Makefile
> +++ b/manual/Makefile
> @@ -58,7 +58,8 @@ chapters = $(addsuffix .texi, \
>            resource setjmp signal startup process job nss  \
> Â Â Â Â Â Â Â Â Â Â Â users sysinfo conf crypt debug)
> Âadd-chapters = $(wildcard $(foreach d, $(add-ons), ../$d/$d.texi))
> -appendices = lang.texi header.texi install.texi maint.texi contrib.texi
> +appendices = lang.texi header.texi install.texi maint.texi platform.texi \
> + Â Â Â Â Â Âcontrib.texi
> Âlicenses = freemanuals.texi lgpl-2.1.texi fdl-1.3.texi
>
> Â-include texis
> diff --git a/manual/contrib.texi b/manual/contrib.texi
> index ed10656..8de97be 100644
> --- a/manual/contrib.texi
> +++ b/manual/contrib.texi
> @@ -1,4 +1,4 @@
> -@node Contributors, Free Manuals, Maintenance, Top
> +@node Contributors, Free Manuals, Platform, Top
> Â@c %MENU% Who wrote what parts of the GNU C Library
> Â@appendix Contributors to @theglibc{}
>
> diff --git a/manual/maint.texi b/manual/maint.texi
> index e1fdbdb..ba22d7c 100644
> --- a/manual/maint.texi
> +++ b/manual/maint.texi
> @@ -1,4 +1,4 @@
> -@node Maintenance, Contributors, Installation, Top
> +@node Maintenance, Platform, Installation, Top
> Â@c %MENU% How to enhance and port the GNU C Library
> Â@appendix Library Maintenance
>
> @@ -104,6 +104,80 @@ This variable is used for secondary object files needed to build
> Â@code{others} or @code{tests}.
> Â@end table
>
> +@menu
> +* Adding Platform-specific Features:: Â ÂAdding platform-specific features.
> +@end menu
> +
> +@node Adding Platform-specific Features
> +@appendixsubsec Platform-specific types, macros and functions
> +
> +It's sometimes necessary to provide nonstandard, platform-specific
> +features to developers. ÂThe C library is traditionally the
> +lowest library layer, so it makes sense for it to provide these
> +low-level features. ÂHowever, including these features in the C
> +library may be a disadvantage if another package provides them
> +as well as there will be two conflicting versions of them. ÂAlso,
> +the features won't be available to projects that do not use
> +@theglibc{} but use other GNU tools, like GCC.
> +
> +The current guidelines are:
> +@itemize @bullet
> +@item
> +If the header provides features that only make sense on a particular machine
> +architecture and have nothing to do with an operating system, then the
> +features should be provided as GCC builtins.
> +
> +@item
> +If the header provides features that are specific to an operating system,
> +both GCC and @theglibc{} could provide it, but @theglibc{} is preferred
> +as it already has a lot of information about the operating system.
> +
> +@item
> +If the header provides features that are specific to an operating system
> +but used by @theglibc{}, then @theglibc{} should provide them.
> +
> +@end itemize
> +
> +The general solution for providing low-level features is to export them as
> +follows:
> +
> +@itemize @bullet
> +@item
> +Nonstandard low-level headers that define macros and static inline
> +functions go in @file{sys/platform/}.
> +
> +@item
> +Each is uniquely named per platform to avoid users thinking they have
> +anything in common e.g., Â@file{sys/platform/@var{arch}.h} or
> +@file{sys/platform/ppc.h}, but not @file{sys/platform.h}.
> +
> +@item
> +The platform header provided by @theglibc{} should coordinate with GCC such
> +that compiler built-in versions of the functions and macros are
> +preferred if available. ÂThis means that user programs will only ever need to
> +include @file{sys/platform/@var{arch}.h}, keeping the same names of types,
> +macros, and functions for convenience and portability.
> +
> +@item
> +Each included symbol must have the prefix @file{__@var{arch}_}, e.g.,
> +@file{__ppc_get_timebase}.
> +
> +@end itemize
> +
> +
> +The easiest way to ship a header is to add it to the @code{sysdep_headers}
> +variable. For example, the combination of Linux-specific headers on
> +PowerPC could be provided like this:
> +
> +@smallexample
> +sysdep_header += sys/platform/ppc.h
> +@end smallexample
> +
> +Then ensure that you have added a @file{sys/platform/ppc.h}
> +header underneath your target platform sysdeps directory, e.g.,
> +@file{sysdeps/powerpc/sys/platform/ppc.h}.
> +
> +
> Â@node Porting
> Â@appendixsec Porting @theglibc{}
>
> diff --git a/manual/platform.texi b/manual/platform.texi
> new file mode 100644
> index 0000000..f893040
> --- /dev/null
> +++ b/manual/platform.texi
> @@ -0,0 +1,28 @@
> +@node Platform, Contributors, Maintenance, Top
> +@c %MENU% Describe all platform-specific facilities provided
> +@appendix Summary of platform-specific facilities
> +
> +@Theglibc{} can provide platform-specific functionality.
> +@ref{Adding Platform-specific Features} describes the process to include
> +these facilities in @theglibc{} and whether they should be included in
> +GCC as well.
> +
> +@menu
> +* PowerPC Architecture Facilities:: Â Â ÂSummary of Facilities Specific
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â to the PowerPC Architecture
> +@end menu
> +
> +@node PowerPC Architecture Facilities
> +@appendixsec Summary of Facilities Specific to the PowerPC Architecture
> +
> +All the facilities specific to the PowerPC Architecture that aren't OS
> +specific are provided by the header @file{sys/platform/ppc.h}.
> +
> +@deftp {Data Type} {__ppc_timebase}
> +This is an unsigned long long type that represents the value read from the
> +timebase register.
> +@end deftp
> +
> +@deftypefun {__ppc_timebase} __ppc_get_timebase (void)
> +Read the timebase register without the need of a system call.
> +@end deftypefun
> diff --git a/sysdeps/powerpc/Makefile b/sysdeps/powerpc/Makefile
> index 23a9a16..54cda7c 100644
> --- a/sysdeps/powerpc/Makefile
> +++ b/sysdeps/powerpc/Makefile
> @@ -26,3 +26,7 @@ gen-as-const-headers += rtld-global-offsets.sym
> Â# get offset to __locale_struct.__ctype_tolower
> Âgen-as-const-headers += locale-defines.sym
> Âendif
> +
> +sysdep_headers += sys/platform/ppc.h
> +
> +tests += test-gettimebase
> diff --git a/sysdeps/powerpc/sys/platform/ppc.h b/sysdeps/powerpc/sys/platform/ppc.h
> new file mode 100644
> index 0000000..af3b9cd
> --- /dev/null
> +++ b/sysdeps/powerpc/sys/platform/ppc.h
> @@ -0,0 +1,53 @@
> +/* Copyright (C) 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
> + Â modify it under the terms of the GNU Lesser General Public
> + Â License as published by the Free Software Foundation; either
> + Â version 2.1 of the License, or (at your option) any later version.
> +
> + Â The GNU C Library is distributed in the hope that it will be useful,
> + Â but WITHOUT ANY WARRANTY; without even the implied warranty of
> + Â MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ÂSee the GNU
> + Â Lesser General Public License for more details.
> +
> + Â You should have received a copy of the GNU Lesser General Public
> + Â License along with the GNU C Library; if not, see
> + Â <http://www.gnu.org/licenses/>. Â*/
> +
> +#ifndef _SYS_PLATFORM_PPC_H
> +#define _SYS_PLATFORM_PPC_H Â Â1
> +
> +typedef unsigned long long int __ppc_timebase;
> +
> +/* Read the Time Base Register
> + Â The Time Base Register is a 64-bit register that stores a monotonically
> + Â incremented value updated at a system-dependent frequency that may be
> + Â different from the processor frequency.
> + Â More information in Power ISA 2.06b - Book II - Section 5.2 Â */
> +static inline __ppc_timebase
> +__ppc_get_timebase (void)
> +{
> +#ifdef __powerpc64__
> + Â__ppc_timebase __tb;
> + Â__asm__ volatile (
> + Â Â Â Â Â Â Â Â Â "mfspr %0, 268\n"
> + Â Â Â Â Â Â Â Â Â : "=r" (__tb)
> + Â Â Â Â Â Â Â Â Â : );
> + Âreturn __tb;
> +#else Â/* not __powerpc64__ */
> + Âregister unsigned long __tbu, __tbl, __tmp; \
> + Â__asm__ volatile (
> + Â Â Â Â Â Â Â Â Â "0:\n"
> + Â Â Â Â Â Â Â Â Â "mftbu %0\n"
> + Â Â Â Â Â Â Â Â Â "mftbl %1\n"
> + Â Â Â Â Â Â Â Â Â "mftbu %2\n"
> + Â Â Â Â Â Â Â Â Â "cmpw %0, %2\n"
> + Â Â Â Â Â Â Â Â Â "bne- 0b\n"
> + Â Â Â Â Â Â Â Â Â : "=r" (__tbu), "=r" (__tbl), "=r" (__tmp)
> + Â Â Â Â Â Â Â Â Â : );
> + Âreturn (( (__ppc_timebase) __tbu << 32) | __tbl);
> +#endif Â/* not __powerpc64__ */
> +}
> +
> +#endif Â/* sys/platform/ppc.h */
> diff --git a/sysdeps/powerpc/test-gettimebase.c b/sysdeps/powerpc/test-gettimebase.c
> new file mode 100644
> index 0000000..0496bd9
> --- /dev/null
> +++ b/sysdeps/powerpc/test-gettimebase.c
> @@ -0,0 +1,46 @@
> +/* Check __ppc_get_timebase() for architecture changes
> + Â Copyright (C) 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
> + Â modify it under the terms of the GNU Lesser General Public
> + Â License as published by the Free Software Foundation; either
> + Â version 2.1 of the License, or (at your option) any later version.
> +
> + Â The GNU C Library is distributed in the hope that it will be useful,
> + Â but WITHOUT ANY WARRANTY; without even the implied warranty of
> + Â MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ÂSee the GNU
> + Â Lesser General Public License for more details.
> +
> + Â You should have received a copy of the GNU Lesser General Public
> + Â License along with the GNU C Library; if not, see
> + Â <http://www.gnu.org/licenses/>. Â*/
> +
> +/* Test if __ppc_get_timebase() is compatible with the current processor and if
> + Â it's changing between reads.
> + Â In case of a read failure it may indicate a future Power ISA or a binutils
> + Â change. Â*/
> +
> +#include <stdio.h>
> +
> +#include <sys/platform/ppc.h>
> +
> +static int
> +do_test (void)
> +{
> + Â__ppc_timebase t1, t2, t3;
> + Ât1 = __ppc_get_timebase ();
> + Âprintf ("Time Base = %llx\n", t1);
> + Ât2 = __ppc_get_timebase ();
> + Âprintf ("Time Base = %llx\n", t2);
> + Ât3 = __ppc_get_timebase ();
> + Âprintf ("Time Base = %llx\n", t3);
> + Âif (t1 != t2 && t1 != t3 && t2 != t3)
> + Â Âreturn 0;
> +
> + Âprintf ("Fail: timebase reads should always be different.");
> + Âreturn 1;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> --
> 1.7.4.4
>

Hi Roland,

Does Tulio's latest patch satisfy your concerns regarding the manual changes?

Ryan