This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fix -Wundef warnins for __FP_FAST_FMA*
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>, libc-alpha at sourceware dot org, Roland McGrath <roland at hack dot frob dot com>, "Joseph S. Myers" <joseph at codesourcery dot com>
- Date: Tue, 18 Mar 2014 00:38:37 -0400
- Subject: Re: [PATCH] Fix -Wundef warnins for __FP_FAST_FMA*
- Authentication-results: sourceware.org; auth=none
- References: <20140318020544 dot GO1850 at spoyarek dot pnq dot redhat dot com>
On 03/17/2014 10:05 PM, Siddhesh Poyarekar wrote:
> The macros are defined by the compiler, so we can only verify whether
> they are defined or not.
We can do more than that.
> I have made changes to the arm and tile bits as well, but I have not
> tested them. OK to commit?
I'm including Roland in this discussion, because I'd like to hear
his input.
I feel like the correction solution to use a configure check to
determine if the compiler defines e.g. __FP_FAST_FMA, and then
explicitly define a compiler feature macro as 0 or 1.
This definitely makes the entire process robust. The configure
script knows what features the compiler has, defines macros for
them, and then you use #if in the code to catch any source of
errors from undefined macros caused by incorrectly configured
compilers etc.
As far as I know this is what we've always wanted e.g. the
configure script checking for compiler features and setting
compiler feature macros rather than testing compiler feature
macros directly.
This isn't always going to be what we want to do though since
for example some source is compiled with different flags that
change compiler defines and that would require a single source
file to include different headers based on compiler options
in order to support using only #if. However, in this case where
we have an explicit compiler feature to test, we should probably
check for that feature directly.
Comments?
> Siddhesh
>
> * bits/mathdef.h: Use #ifdef instead of #if.
> * sysdeps/arm/bits/mathdef.h [defined __USE_ISOC99 && defined
> _MATH_H && !defined _MATH_H_MATHDEF]: Likewise.
> * sysdeps/tile/bits/mathdef.h [defined __USE_ISOC99 && defined
> _MATH_H && !defined _MATH_H_MATHDEF]: Likewise.
> * sysdeps/x86/bits/mathdef.h [defined __USE_ISOC99 && defined
> _MATH_H && !defined _MATH_H_MATHDEF]: Likewise.
>
>
> ---
> bits/mathdef.h | 6 +++---
> sysdeps/arm/bits/mathdef.h | 6 +++---
> sysdeps/tile/bits/mathdef.h | 6 +++---
> sysdeps/x86/bits/mathdef.h | 6 +++---
> 4 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/bits/mathdef.h b/bits/mathdef.h
> index ca1f464..f27ecac 100644
> --- a/bits/mathdef.h
> +++ b/bits/mathdef.h
> @@ -35,15 +35,15 @@ typedef double double_t; /* `double' expressions are evaluated as
>
> /* The GCC 4.6 compiler will define __FP_FAST_FMA{,F,L} if the fma{,f,l}
> builtins are supported. */
> -#if __FP_FAST_FMA
> +#ifdef __FP_FAST_FMA
> # define FP_FAST_FMA 1
> #endif
>
> -#if __FP_FAST_FMAF
> +#ifdef __FP_FAST_FMAF
> # define FP_FAST_FMAF 1
> #endif
>
> -#if __FP_FAST_FMAL
> +#ifdef __FP_FAST_FMAL
> # define FP_FAST_FMAL 1
> #endif
>
> diff --git a/sysdeps/arm/bits/mathdef.h b/sysdeps/arm/bits/mathdef.h
> index be727e5..f309002 100644
> --- a/sysdeps/arm/bits/mathdef.h
> +++ b/sysdeps/arm/bits/mathdef.h
> @@ -34,15 +34,15 @@ typedef double double_t; /* `double' expressions are evaluated as
>
> /* The GCC 4.6 compiler will define __FP_FAST_FMA{,F,L} if the fma{,f,l}
> builtins are supported. */
> -# if __FP_FAST_FMA
> +# ifdef __FP_FAST_FMA
> # define FP_FAST_FMA 1
> # endif
>
> -# if __FP_FAST_FMAF
> +# ifdef __FP_FAST_FMAF
> # define FP_FAST_FMAF 1
> # endif
>
> -# if __FP_FAST_FMAL
> +# ifdef __FP_FAST_FMAL
> # define FP_FAST_FMAL 1
> # endif
>
> diff --git a/sysdeps/tile/bits/mathdef.h b/sysdeps/tile/bits/mathdef.h
> index d043b4a..c26a2e7 100644
> --- a/sysdeps/tile/bits/mathdef.h
> +++ b/sysdeps/tile/bits/mathdef.h
> @@ -33,15 +33,15 @@ typedef double double_t;
>
> /* The GCC 4.6 compiler will define __FP_FAST_FMA{,F,L} if the fma{,f,l}
> builtins are supported. */
> -# if __FP_FAST_FMA
> +# ifdef __FP_FAST_FMA
> # define FP_FAST_FMA 1
> # endif
>
> -# if __FP_FAST_FMAF
> +# ifdef __FP_FAST_FMAF
> # define FP_FAST_FMAF 1
> # endif
>
> -# if __FP_FAST_FMAL
> +# ifdef __FP_FAST_FMAL
> # define FP_FAST_FMAL 1
> # endif
>
> diff --git a/sysdeps/x86/bits/mathdef.h b/sysdeps/x86/bits/mathdef.h
> index 07c2d66..fd9cf42 100644
> --- a/sysdeps/x86/bits/mathdef.h
> +++ b/sysdeps/x86/bits/mathdef.h
> @@ -44,15 +44,15 @@ typedef long double double_t; /* `double' expressions are evaluated as
>
> /* The GCC 4.6 compiler will define __FP_FAST_FMA{,F,L} if the fma{,f,l}
> builtins are supported. */
> -# if __FP_FAST_FMA
> +# ifdef __FP_FAST_FMA
> # define FP_FAST_FMA 1
> # endif
>
> -# if __FP_FAST_FMAF
> +# ifdef __FP_FAST_FMAF
> # define FP_FAST_FMAF 1
> # endif
>
> -# if __FP_FAST_FMAL
> +# ifdef __FP_FAST_FMAL
> # define FP_FAST_FMAL 1
> # endif
>
>