[Mesa-dev] [PATCH] mesa: Fix build to properly check for supported compiler flags

[Matt Turner]

On Tue, Dec 17, 2013 at 7:15 AM, Lauri Kasanen <cand at gmx.com> wrote:
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=72708

Make this "Bugzilla: ..."

> It seems to me that the Intel code that uses this SSE4.1 function
> is still buggy, as it has no runtime check - would it not crash
> if built on a SSE4-capable system but used with a lower-class cpu?

You haven't thought this through. :)

The SSE 4.1 code is in i965 #ifdef __SSE4_1__. __SSE4_1__ is defined
if the code is compiled with the appropriate CFLAGS, e.g., -msse4.1 or
-march=native and your CPU support SSE 4.1.

That is, your Mesa would have to be compiled with flags incompatible
with your CPU for this to break, and in that case gcc would already
have used instructions your CPU doesn't have.

> i965 systems = LGA775 = Pentium4 without SSE4.
>
> Anyway, NMI.
>
> Signed-off-by: Lauri Kasanen <cand at gmx.com>
> ---
>  configure.ac                |  6 ++++
>  m4/ax_check_compile_flag.m4 | 72 +++++++++++++++++++++++++++++++++++++++++++++
>  src/mesa/Makefile.am        |  6 +++-
>  3 files changed, 83 insertions(+), 1 deletion(-)
>  create mode 100644 m4/ax_check_compile_flag.m4
>
> diff --git a/configure.ac b/configure.ac
> index 6481627..3944084 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -231,6 +231,12 @@ AC_SUBST([VISIBILITY_CFLAGS])
>  AC_SUBST([VISIBILITY_CXXFLAGS])
>
>  dnl
> +dnl Optional flags, check for compiler support
> +dnl
> +AX_CHECK_COMPILE_FLAG([-msse4.1], [SSE41_SUPPORTED=1], [SSE41_SUPPORTED=0])
> +AM_CONDITIONAL([SSE41_SUPPORTED], [test x$SSE41_SUPPORTED = x1])
> +
> +dnl
>  dnl Hacks to enable 32 or 64 bit build
>  dnl
>  AC_ARG_ENABLE([32-bit],
> diff --git a/m4/ax_check_compile_flag.m4 b/m4/ax_check_compile_flag.m4
> new file mode 100644
> index 0000000..c3a8d69
> --- /dev/null
> +++ b/m4/ax_check_compile_flag.m4
> @@ -0,0 +1,72 @@
> +# ===========================================================================
> +#   http://www.gnu.org/software/autoconf-archive/ax_check_compile_flag.html
> +# ===========================================================================
> +#
> +# SYNOPSIS
> +#
> +#   AX_CHECK_COMPILE_FLAG(FLAG, [ACTION-SUCCESS], [ACTION-FAILURE], [EXTRA-FLAGS])
> +#
> +# DESCRIPTION
> +#
> +#   Check whether the given FLAG works with the current language's compiler
> +#   or gives an error.  (Warnings, however, are ignored)
> +#
> +#   ACTION-SUCCESS/ACTION-FAILURE are shell commands to execute on
> +#   success/failure.
> +#
> +#   If EXTRA-FLAGS is defined, it is added to the current language's default
> +#   flags (e.g. CFLAGS) when the check is done.  The check is thus made with
> +#   the flags: "CFLAGS EXTRA-FLAGS FLAG".  This can for example be used to
> +#   force the compiler to issue an error when a bad flag is given.
> +#
> +#   NOTE: Implementation based on AX_CFLAGS_GCC_OPTION. Please keep this
> +#   macro in sync with AX_CHECK_{PREPROC,LINK}_FLAG.
> +#
> +# LICENSE
> +#
> +#   Copyright (c) 2008 Guido U. Draheim <guidod at gmx.de>
> +#   Copyright (c) 2011 Maarten Bosmans <mkbosmans at gmail.com>
> +#
> +#   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.
> +
> +#serial 2
> +
> +AC_DEFUN([AX_CHECK_COMPILE_FLAG],
> +[AC_PREREQ(2.59)dnl for _AC_LANG_PREFIX
> +AS_VAR_PUSHDEF([CACHEVAR],[ax_cv_check_[]_AC_LANG_ABBREV[]flags_$4_$1])dnl
> +AC_CACHE_CHECK([whether _AC_LANG compiler accepts $1], CACHEVAR, [
> +  ax_check_save_flags=$[]_AC_LANG_PREFIX[]FLAGS
> +  _AC_LANG_PREFIX[]FLAGS="$[]_AC_LANG_PREFIX[]FLAGS $4 $1"
> +  AC_COMPILE_IFELSE([AC_LANG_PROGRAM()],
> +    [AS_VAR_SET(CACHEVAR,[yes])],
> +    [AS_VAR_SET(CACHEVAR,[no])])
> +  _AC_LANG_PREFIX[]FLAGS=$ax_check_save_flags])
> +AS_IF([test x"AS_VAR_GET(CACHEVAR)" = xyes],
> +  [m4_default([$2], :)],
> +  [m4_default([$3], :)])
> +AS_VAR_POPDEF([CACHEVAR])dnl
> +])dnl AX_CHECK_COMPILE_FLAGS
> diff --git a/src/mesa/Makefile.am b/src/mesa/Makefile.am
> index a60600e..8843836 100644
> --- a/src/mesa/Makefile.am
> +++ b/src/mesa/Makefile.am
> @@ -103,7 +103,11 @@ noinst_PROGRAMS = gen_matypes
>  gen_matypes_SOURCES = x86/gen_matypes.c
>  BUILT_SOURCES += matypes.h
>
> -ARCH_LIBS = libmesa_sse41.la
> +ARCH_LIBS =
> +
> +if SSE41_SUPPORTED
> +ARCH_LIBS += libmesa_sse41.la
> +endif

It's safe to just assign ARCH_LIBS once and not do the empty assignment.

With that fixed and most of the commit message removed,

Reviewed-by: Matt Turner <mattst88 at gmail.com>

(send an updated patch and I'll commit it)