[LLVMdev] [Mesa-dev] [PATCH 1/2 v3] configure: add visibility macro detection to configure

[Emil Velikov]

Hi Marc

On 17/02/15 09:40, Marc Dietrich wrote:
> This adds clang/gcc visibility macro detection to configure and util/macros.h.
> This is can be used to conveniently add e.g. a "HIDDEN" attribute to a function.
> 
I believe this should be OK to go in regardless of the status of patch
2. There are just a couple of trivial nitpicks.

> Signed-off-by: Marc Dietrich <marvin24 at gmx.de>
> ---
> v2: use VISIBILITY_*FLAGS instead of *FLAGS directly
> v3: no change
> 
>  configure.ac      | 28 ++++++----------------------
>  src/util/macros.h |  6 ++++++
>  2 files changed, 12 insertions(+), 22 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 351027b..266764a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -189,6 +189,7 @@ AX_GCC_FUNC_ATTRIBUTE([flatten])
>  AX_GCC_FUNC_ATTRIBUTE([format])
>  AX_GCC_FUNC_ATTRIBUTE([malloc])
>  AX_GCC_FUNC_ATTRIBUTE([packed])
> +AX_GCC_FUNC_ATTRIBUTE([visibility])
>  
>  AM_CONDITIONAL([GEN_ASM_OFFSETS], test "x$GEN_ASM_OFFSETS" = xyes)
>  
> @@ -245,17 +246,13 @@ if test "x$GCC" = xyes; then
>  		   AC_MSG_RESULT([yes]),
>  		   [CFLAGS="$save_CFLAGS -Wmissing-prototypes";
>  		    AC_MSG_RESULT([no])]);
> +    CFLAGS=$save_CFLAGS
>  
I'm not sure we want/need this one ?

>      # Enable -fvisibility=hidden if using a gcc that supports it
As spotted by Sedat, please update the comment.

> -    save_CFLAGS="$CFLAGS"
> -    AC_MSG_CHECKING([whether $CC supports -fvisibility=hidden])
> -    VISIBILITY_CFLAGS="-fvisibility=hidden"
> -    CFLAGS="$CFLAGS $VISIBILITY_CFLAGS"
> -    AC_LINK_IFELSE([AC_LANG_PROGRAM()], AC_MSG_RESULT([yes]),
> -		   [VISIBILITY_CFLAGS=""; AC_MSG_RESULT([no])]);
> -
> -    # Restore CFLAGS; VISIBILITY_CFLAGS are added to it where needed.
> -    CFLAGS=$save_CFLAGS
> +    if test "x${ax_cv_have_func_attribute_visibility}" = xyes; then
> +	VISIBILITY_CFLAGS="-fvisibility=hidden"
> +	VISIBILITY_CXXFLAGS="-fvisibility=hidden"
> +    fi
>  
As these two are no longer GCC "specific" we can move them out of the if
test x$GCC = xyes, conditional.


>      # Work around aliasing bugs - developers should comment this out
>      CFLAGS="$CFLAGS -fno-strict-aliasing"
> @@ -267,19 +264,6 @@ fi
>  if test "x$GXX" = xyes; then
>      CXXFLAGS="$CXXFLAGS -Wall"
>  
> -    # Enable -fvisibility=hidden if using a gcc that supports it
> -    save_CXXFLAGS="$CXXFLAGS"
> -    AC_MSG_CHECKING([whether $CXX supports -fvisibility=hidden])
> -    VISIBILITY_CXXFLAGS="-fvisibility=hidden"
> -    CXXFLAGS="$CXXFLAGS $VISIBILITY_CXXFLAGS"
> -    AC_LANG_PUSH([C++])
> -    AC_LINK_IFELSE([AC_LANG_PROGRAM()], AC_MSG_RESULT([yes]),
> -		   [VISIBILITY_CXXFLAGS="" ; AC_MSG_RESULT([no])]);
> -    AC_LANG_POP([C++])
> -
> -    # Restore CXXFLAGS; VISIBILITY_CXXFLAGS are added to it where needed.
> -    CXXFLAGS=$save_CXXFLAGS
> -
>      # Work around aliasing bugs - developers should comment this out
>      CXXFLAGS="$CXXFLAGS -fno-strict-aliasing"
>  
> diff --git a/src/util/macros.h b/src/util/macros.h
> index eec8b93..7682511 100644
> --- a/src/util/macros.h
> +++ b/src/util/macros.h
> @@ -117,6 +117,12 @@ do {                       \
>  #define PRINTFLIKE(f, a)
>  #endif
>  
> +#ifdef HAVE_FUNC_ATTRIBUTE_VISIBILITY
> +#define HIDDEN __attribute__((visibility("hidden")))
> +#else
> +#define HIDDEN
> +#endif
> +
We already define HIDDEN in the asm code.

Can you check (build or otherwise) that things don't clash for
--enable-asm and --{en,dis}able-shared-glapi. Check the configure log,
as we conveniently toggle the latter.

Thanks
Emil