[Mesa-dev] [PATCH V2] mesa: use build flag to ensure stack is realigned on x86

[Matt Turner]

On Sun, Dec 7, 2014 at 4:13 AM, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> Nowadays GCC assumes stack pointer is 16-byte aligned even on 32-bits, but that is an assumption OpenGL drivers (or any dynamic library for that matter) can't afford to make as there are many closed- and open- source application binaries out there that only assume 4-byte stack alignment.

Line wrap the commit message.

>
> V2: use $target_cpu rather than $host_cpu
>  and setup build flags in config rather than makefile
>
> https://bugs.freedesktop.org/show_bug.cgi?id=86788
> Signed-off-by: Timothy Arceri <t_arceri at yahoo.com.au>
> ---
>  Tested by cross compiling and running 32-bit version of
>  UrbanTerror.
>
>  Please note if this patch is ok it should also be applied to 10.4 with
>  the last hunk removed.
>
>  configure.ac               | 11 ++++++++++-
>  src/mesa/Makefile.am       |  2 +-
>  src/mesa/main/sse_minmax.c |  3 ---
>  3 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index b0df1bb..7dc435a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -253,8 +253,9 @@ AC_SUBST([VISIBILITY_CXXFLAGS])
>  dnl
>  dnl Optional flags, check for compiler support
>  dnl
> +SSE41_CFLAGS="-msse4.1"
>  save_CFLAGS="$CFLAGS"
> -CFLAGS="-msse4.1 $CFLAGS"
> +CFLAGS="$SSE41_CFLAGS $CFLAGS"
>  AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
>  #include <smmintrin.h>
>  int main () {
> @@ -474,6 +475,12 @@ fi
>  dnl
>  dnl Arch/platform-specific settings
>  dnl
> +case "$target_cpu" in
> +    i?86)
> +        SSE41_CFLAGS="$SSE41_CFLAGS -mstackrealign"
> +;;

;; should be indented.

> +esac

I'd put this immediately after the SSE41_CFLAGS="..." assignment so
that we're compiling the test program with -mstackrealign as well.

> +
>  AC_ARG_ENABLE([asm],
>      [AS_HELP_STRING([--disable-asm],
>          [disable assembly usage @<:@default=enabled on supported plaforms@:>@])],
> @@ -2091,6 +2098,8 @@ AM_CONDITIONAL(HAVE_X86_ASM, test "x$asm_arch" = xx86 -o "x$asm_arch" = xx86_64)
>  AM_CONDITIONAL(HAVE_X86_64_ASM, test "x$asm_arch" = xx86_64)
>  AM_CONDITIONAL(HAVE_SPARC_ASM, test "x$asm_arch" = xsparc)
>
> +AC_SUBST([SSE41_CFLAGS], $SSE41_CFLAGS)

I'd put this immediately before or after the
AM_CONDITIONAL([SSE41_SUPPORTED], ...) statement.

With those comments addressed, it's

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

Thanks Timothy!