Re: [Patch] Fix HP_SMALL_TIMING_AVAIL undef warnings

[Will Newton <will dot newton at linaro dot org>]

On 30 April 2014 23:33, Steve Ellcey <sellcey@mips.com> wrote:

> Here is a new *untested* patch for the HP_SMALL_TIMING_AVAIL warnings but
> the patch actually does much more then get rid of these warnings.  There
> was a lot of duplication in the various platform hp-timing.h header files,
> but these were all different then the generic hp-timing.h header file which
> assumes you are not implementing any timing functionality.  So I created
> a generic hp-timing-enabled.h header file, put the macros in there and
> had the various platform files include that.
>
> The two macros I did not define in hp-timing-enabled.h are HP_TIMING_NOW
> and HP_TIMING_ACCUM.  These are pretty different across all the targets
> and so I left each target to define its own.  I also couldn't put the
> typedef of hp_timing_t in the shared file because it is different for
> one target (alpha), where it is 32 bits instead of 64 like it is on the
> other targets, but I did create a default HP_TIMING_TYPE that could
> be used in the hp_timing_t typedef declaration for each target and only
> alpha had to redefine it.
>
> This isn't ready for checkin since it isn't tested (I did do a build on
> x86) but I was hoping someone could review it for design issues and
> that the various platform maintainers for the platforms affected by the
> change (x86, alpha, sparc, ia64, powerpc) could try it out.
>
> Steve Ellcey
> sellcey@mips.com
>
>
> 2014-04-30  Steve Ellcey  <sellcey@mips.com>
>
>         * sysdeps/generic/hp-timing-enabled.h: New file.
>         * sysdeps/generic/hp-timing.h (HP_SMALL_TIMING_AVAIL):
>         Set default value.
>         * sysdeps/powerpc/powerpc32/hp-timing.h (HP_SMALL_TIMING_AVAIL): Ditto.
>         * sysdeps/alpha/hp-timing.h: Include hp-timing-enabled.h to get
>         default defintions of HP_TIMING_* macros.

Typo definitions.

>         * sysdeps/i386/i686/hp-timing.h: Ditto.
>         * sysdeps/ia64/hp-timing.h: Ditto.
>         * sysdeps/powerpc/powerpc32/power4/hp-timing.h: Ditto.
>         * sysdeps/powerpc/powerpc64/hp-timing.h: Ditto.
>         * sysdeps/sparc/sparc32/sparcv9/hp-timing.h: Ditto.
>         * sysdeps/sparc/sparc64/hp-timing.h: Ditto.
>         * elf/dl-support.c: Use #if to check HP_SMALL_TIMING_AVAIL.
>         * elf/rtld.c: Ditto.

Sorry for taking so long to look at this. A couple of things:

HP_TIMING_MAX is documented but it's actually HP_TIMING_TYPE_MAX. I
wonder whether it might be as well to always make every architecture
define hp_timing_t and HP_TIMING_TYPE_MAX at the cost of a little
duplication (or we could make it ~(hp_timing_t)0?).

The original warnings are fixed but new ones are added of the form:

 In file included from ../sysdeps/x86_64/hp-timing.h:22:0,
                 from ../include/libc-internal.h:7,
                 from ../sysdeps/x86_64/nptl/tls.h:29,
                 from ../sysdeps/unix/sysv/linux/x86_64/sysdep.h:23,
                 from <stdin>:1:
../sysdeps/i386/i686/hp-timing.h:38:0: warning: "HP_TIMING_ACCUM"
redefined [enabled by default]
 #define HP_TIMING_ACCUM(Sum, Diff) \
 ^
In file included from ../sysdeps/i386/i686/hp-timing.h:26:0,
                 from ../sysdeps/x86_64/hp-timing.h:22,
                 from ../include/libc-internal.h:7,
                 from ../sysdeps/x86_64/nptl/tls.h:29,
                 from ../sysdeps/unix/sysv/linux/x86_64/sysdep.h:23,
                 from <stdin>:1:

-- 
Will Newton
Toolchain Working Group, Linaro