[PATCH v3] glx: Silence warnings when building with clang

Jamey Sharp jamey at minilop.net
Mon Apr 25 13:38:34 PDT 2011


I approve of this patch in principle, so I tested it. GCC, at least in
version 4.5.2, reports "error: -Werror=unknown-attributes: No option
-Wunknown-attributes", so it doesn't detect support for tls_model.

If I change xorg_tls.m4 to use "-Werror=attributes" instead, I can
confirm that it detects tls_model, and that the code in glx/ compiles
without error using the new definition of the TLS macro.

I like the implementation, aside from the detail that it doesn't quite
work :-) so I'll happily give a reviewed-by for a patch that I can
confirm works with my version of GCC.

It probably needs review on at least Solaris as well, so CC'ing Alan,
and ideally whichever OS is still building on GCC 2.95 would test too.

Jamey

On Mon, Apr 25, 2011 at 12:20:16PM -0700, Jeremy Huddleston wrote:
> This replaces AX_TLS (GPL3) with XORG_TLS (MIT)
> 
> In file included from glapi.c:46:
> In file included from ./glapi.h:51:
> ./glthread.h:237:20: error: unknown attribute 'tls_model' ignored [-Werror,-Wunknown-attributes]
>     __attribute__((tls_model("initial-exec")));
>                    ^
> In file included from glapi.c:46:
> ./glapi.h:92:20: error: unknown attribute 'tls_model' ignored [-Werror,-Wunknown-attributes]
>     __attribute__((tls_model("initial-exec")));
>                    ^
> glapi.c:82:20: error: unknown attribute 'tls_model' ignored [-Werror,-Wunknown-attributes]
>     __attribute__((tls_model("initial-exec"))) = NULL;
>                    ^
> glapi.c:85:20: error: unknown attribute 'tls_model' ignored [-Werror,-Wunknown-attributes]
>     __attribute__((tls_model("initial-exec")));
>                    ^
> 4 errors generated.
> 
> Signed-off-by: Jeremy Huddleston <jeremyhu at apple.com>
> ---
>  configure.ac            |    3 +-
>  glx/glapi.c             |    6 +--
>  glx/glapi.h             |    3 +-
>  glx/glthread.h          |    3 +-
>  include/dix-config.h.in |    2 +-
>  m4/ax_tls.m4            |   74 -----------------------------------------------
>  m4/xorg-tls.m4          |   54 ++++++++++++++++++++++++++++++++++
>  7 files changed, 61 insertions(+), 84 deletions(-)
>  delete mode 100644 m4/ax_tls.m4
>  create mode 100644 m4/xorg-tls.m4
> 
> diff --git a/configure.ac b/configure.ac
> index 86e67f0..80f4d0d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -590,7 +590,8 @@ dnl GLX build options
>  AC_ARG_ENABLE(aiglx,          AS_HELP_STRING([--enable-aiglx], [Build accelerated indirect GLX (default: enabled)]),
>                                  [AIGLX=$enableval],
>                                  [AIGLX=yes])
> -AX_TLS
> +
> +XORG_TLS
>  AC_ARG_ENABLE(glx-tls,        AS_HELP_STRING([--enable-glx-tls], [Build GLX with TLS support (default: auto)]),
>                                  [GLX_USE_TLS=$enableval
>                                   if test "x$GLX_USE_TLS" = "xyes" && test "${ac_cv_tls}" = "none" ; then
> diff --git a/glx/glapi.c b/glx/glapi.c
> index 7cb8495..9e219f6 100644
> --- a/glx/glapi.c
> +++ b/glx/glapi.c
> @@ -78,11 +78,9 @@ static void init_glapi_relocs(void);
>  /*@{*/
>  #if defined(GLX_USE_TLS)
>  
> -PUBLIC TLS struct _glapi_table * _glapi_tls_Dispatch
> -    __attribute__((tls_model("initial-exec"))) = NULL;
> +PUBLIC TLS struct _glapi_table * _glapi_tls_Dispatch = NULL;
>  
> -PUBLIC TLS void * _glapi_tls_Context
> -    __attribute__((tls_model("initial-exec")));
> +PUBLIC TLS void * _glapi_tls_Context;
>  
>  PUBLIC const struct _glapi_table *_glapi_Dispatch = NULL;
>  PUBLIC const void *_glapi_Context = NULL;
> diff --git a/glx/glapi.h b/glx/glapi.h
> index 6521f31..7051c1e 100644
> --- a/glx/glapi.h
> +++ b/glx/glapi.h
> @@ -83,8 +83,7 @@ typedef void (*_glapi_warning_func)(void *ctx, const char *str, ...);
>  const extern void *_glapi_Context;
>  const extern struct _glapi_table *_glapi_Dispatch;
>  
> -extern TLS void * _glapi_tls_Context
> -    __attribute__((tls_model("initial-exec")));
> +extern TLS void * _glapi_tls_Context;
>  
>  # define GET_CURRENT_CONTEXT(C)  GLcontext *C = (GLcontext *) _glapi_tls_Context
>  
> diff --git a/glx/glthread.h b/glx/glthread.h
> index 140e2aa..532401a 100644
> --- a/glx/glthread.h
> +++ b/glx/glthread.h
> @@ -233,8 +233,7 @@ _glthread_SetTSD(_glthread_TSD *, void *);
>  
>  #if defined(GLX_USE_TLS)
>  
> -extern TLS struct _glapi_table * _glapi_tls_Dispatch
> -    __attribute__((tls_model("initial-exec")));
> +extern TLS struct _glapi_table * _glapi_tls_Dispatch;
>  
>  #define GET_DISPATCH() _glapi_tls_Dispatch
>  
> diff --git a/include/dix-config.h.in b/include/dix-config.h.in
> index f00c767..4710ef8 100644
> --- a/include/dix-config.h.in
> +++ b/include/dix-config.h.in
> @@ -444,7 +444,7 @@
>  /* Define to 1 if you have the `ffs' function. */
>  #undef HAVE_FFS
>  
> -/* If the compiler supports a TLS storage class define it to that here */
> +/* The compiler supported TLS storage class, prefering initial-exec if tls_model is supported */
>  #undef TLS
>  
>  /* Correctly set _XSERVER64 for OSX fat binaries */
> diff --git a/m4/ax_tls.m4 b/m4/ax_tls.m4
> deleted file mode 100644
> index 481c3d0..0000000
> --- a/m4/ax_tls.m4
> +++ /dev/null
> @@ -1,74 +0,0 @@
> -# ===========================================================================
> -#             http://www.nongnu.org/autoconf-archive/ax_tls.html
> -# ===========================================================================
> -#
> -# SYNOPSIS
> -#
> -#   AX_TLS
> -#
> -# DESCRIPTION
> -#
> -#   Provides a test for the compiler support of thread local storage (TLS)
> -#   extensions. Defines TLS if it is found. Currently only knows about GCC
> -#   and MSVC. I think SunPro uses the same as GCC, and Borland apparently
> -#   supports either.
> -#
> -# LICENSE
> -#
> -#   Copyright (c) 2008 Alan Woodland <ajw05 at aber.ac.uk>
> -#
> -#   This program is free software: you can redistribute it and/or modify it
> -#   under the terms of the GNU General Public License as published by the
> -#   Free Software Foundation, either version 3 of the License, or (at your
> -#   option) any later version.
> -#
> -#   This program 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 General
> -#   Public License for more details.
> -#
> -#   You should have received a copy of the GNU General Public License along
> -#   with this program. If not, see <http://www.gnu.org/licenses/>.
> -#
> -#   As a special exception, the respective Autoconf Macro's copyright owner
> -#   gives unlimited permission to copy, distribute and modify the configure
> -#   scripts that are the output of Autoconf when processing the Macro. You
> -#   need not follow the terms of the GNU General Public License when using
> -#   or distributing such scripts, even though portions of the text of the
> -#   Macro appear in them. The GNU General Public License (GPL) does govern
> -#   all other use of the material that constitutes the Autoconf Macro.
> -#
> -#   This special exception to the GPL applies to versions of the Autoconf
> -#   Macro released by the Autoconf Archive. When you make and distribute a
> -#   modified version of the Autoconf Macro, you may extend this special
> -#   exception to the GPL to apply to your modified version as well.
> -
> -AC_DEFUN([AX_TLS], [
> -  AC_MSG_CHECKING(for thread local storage (TLS) class)
> -  AC_CACHE_VAL(ac_cv_tls, [
> -    ax_tls_keywords="__thread __declspec(thread) none"
> -    for ax_tls_keyword in $ax_tls_keywords; do
> -       case $ax_tls_keyword in
> -          none) ac_cv_tls=none ; break ;;
> -          *)
> -             AC_TRY_COMPILE(
> -                [#include <stdlib.h>
> -                 static void
> -                 foo(void) {
> -                 static ] $ax_tls_keyword [ int bar;
> -                 exit(1);
> -                 }],
> -                 [],
> -                 [ac_cv_tls=$ax_tls_keyword ; break],
> -                 ac_cv_tls=none
> -             )
> -       esac
> -    done
> -])
> -
> -  if test "$ac_cv_tls" != "none"; then
> -    dnl AC_DEFINE([TLS], [], [If the compiler supports a TLS storage class define it to that here])
> -    AC_DEFINE_UNQUOTED([TLS], $ac_cv_tls, [If the compiler supports a TLS storage class define it to that here])
> -  fi
> -  AC_MSG_RESULT($ac_cv_tls)
> -])
> diff --git a/m4/xorg-tls.m4 b/m4/xorg-tls.m4
> new file mode 100644
> index 0000000..ec2f0fd
> --- /dev/null
> +++ b/m4/xorg-tls.m4
> @@ -0,0 +1,54 @@
> +dnl Copyright © 2011 Apple Inc.
> +dnl
> +dnl Permission is hereby granted, free of charge, to any person obtaining a
> +dnl copy of this software and associated documentation files (the "Software"),
> +dnl to deal in the Software without restriction, including without limitation
> +dnl the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +dnl and/or sell copies of the Software, and to permit persons to whom the
> +dnl Software is furnished to do so, subject to the following conditions:
> +dnl
> +dnl The above copyright notice and this permission notice (including the next
> +dnl paragraph) shall be included in all copies or substantial portions of the
> +dnl Software.
> +dnl
> +dnl THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +dnl IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +dnl FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +dnl THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +dnl LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +dnl FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +dnl DEALINGS IN THE SOFTWARE.
> +dnl
> +dnl Authors: Jeremy Huddleston <jeremyhu at apple.com>
> +
> +AC_DEFUN([XORG_TLS], [
> +    AC_MSG_CHECKING(for thread local storage (TLS) support)
> +    AC_CACHE_VAL(ac_cv_tls, [
> +        ac_cv_tls=none
> +        keywords="__thread __declspec(thread)"
> +        for kw in $keywords ; do
> +            AC_TRY_COMPILE([int $kw test;], [], ac_cv_tls=$kw)
> +        done
> +    ])
> +    AC_MSG_RESULT($ac_cv_tls)
> +
> +    if test "$ac_cv_tls" != "none"; then
> +        AC_MSG_CHECKING(for tls_model attribute support)
> +        AC_CACHE_VAL(ac_cv_tls_model, [
> +            save_CFLAGS="$CFLAGS"
> +            CFLAGS="$CFLAGS -Werror=unknown-attributes"
> +            AC_TRY_COMPILE([int $ac_cv_tls __attribute__((tls_model("initial-exec"))) test;], [],
> +                           ac_cv_tls_model=yes, ac_cv_tls_model=no)
> +            CFLAGS="$save_CFLAGS"
> +        ])
> +        AC_MSG_RESULT($ac_cv_tls_model)
> +
> +        if test "x$ac_cv_tls_model" = "xyes" ; then
> +            xorg_tls=$ac_cv_tls' __attribute__((tls_model("initial-exec")))'
> +        else
> +            xorg_tls=$ac_cv_tls
> +        fi
> +
> +        AC_DEFINE_UNQUOTED([TLS], $xorg_tls, [The compiler supported TLS storage class, prefering initial-exec if tls_model is supported])
> +    fi
> +])
> -- 
> 1.7.4.1
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110425/16f5a9d8/attachment.pgp>


More information about the xorg-devel mailing list