[Mesa-dev] [PATCH] swr: Refactor checks for compiler feature flags

[Rowley, Timothy O]

> On Jun 28, 2016, at 8:24 AM, Chuck Atkins <chuck.atkins at kitware.com> wrote:
> 
> Encapsulate the test for which flags are needed to get a compiler to
> support certain features.  Along with this, give various options to try
> for AVX and AVX2 support.  Ideally we want to use specific instruction
> set feature flags, like -mavx2 for instance instead of -march=haswell,
> but the flags required for certain compilers are different.  This
> allows, for AVX2 for instance, GCC to use -mavx2 -mfma -mbmi2 -mf16c
> while the Intel compiler which doesn't support those flags can fall
> back to using -march=core-avx2.
> 
> This addresses a bug where the Intel compiler will silently ignore the
> AVX2 instruction feature flags and then potentially fail to build.
> 
> Cc: Tim Rowley <timothy.o.rowley at intel.com>
> Signed-off-by: Chuck Atkins <chuck.atkins at kitware.com>
> ---
> configure.ac | 86 +++++++++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 62 insertions(+), 24 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index cc9bc47..806850e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2330,6 +2330,39 @@ swr_llvm_check() {
>     fi
> }
> 
> +swr_cxx_feature_flags_check() {
> +    ifndef_test=$1
> +    option_list="$2"
> +    unset SWR_CXX_FEATURE_FLAGS
> +    AC_LANG_PUSH([C++])
> +    save_CXXFLAGS="$CXXFLAGS"
> +    save_IFS="$IFS"
> +    IFS=","
> +    found=0
> +    for opts in $option_list
> +    do
> +        unset IFS
> +        CXXFLAGS="$opts $save_CXXFLAGS"
> +        AC_COMPILE_IFELSE(
> +            [AC_LANG_PROGRAM(
> +                [   $ifndef_test
> +                    #error
> +                    #endif
> +                ])],
> +            [found=1; break],
> +            [])
> +        IFS=","
> +    done
> +    IFS="$save_IFS"
> +    CXXFLAGS="$save_CXXFLAGS"
> +    AC_LANG_POP([C++])
> +    if test $found -eq 1; then
> +        SWR_CXX_FEATURE_FLAGS="$opts"
> +        return 0
> +    fi
> +    return 1
> +}
> +
> dnl Duplicates in GALLIUM_DRIVERS_DIRS are removed by sorting it after this block
> if test -n "$with_gallium_drivers"; then
>     gallium_drivers=`IFS=', '; echo $with_gallium_drivers`
> @@ -2399,31 +2432,36 @@ if test -n "$with_gallium_drivers"; then
>         xswr)
>             swr_llvm_check "swr"
> 
> -            AC_MSG_CHECKING([whether $CXX supports c++11/AVX/AVX2])
> -            SWR_AVX_CXXFLAGS="-mavx"
> -            SWR_AVX2_CXXFLAGS="-mavx2 -mfma -mbmi2 -mf16c"
> -
> -            AC_LANG_PUSH([C++])
> -            save_CXXFLAGS="$CXXFLAGS"
> -            CXXFLAGS="-std=c++11 $CXXFLAGS"
> -            AC_COMPILE_IFELSE([AC_LANG_PROGRAM()],[],
> -                              [AC_MSG_ERROR([c++11 compiler support not detected])])
> -            CXXFLAGS="$save_CXXFLAGS"
> -
> -            save_CXXFLAGS="$CXXFLAGS"
> -            CXXFLAGS="$SWR_AVX_CXXFLAGS $CXXFLAGS"
> -            AC_COMPILE_IFELSE([AC_LANG_PROGRAM()],[],
> -                              [AC_MSG_ERROR([AVX compiler support not detected])])
> -            CXXFLAGS="$save_CXXFLAGS"
> -
> -            save_CFLAGS="$CXXFLAGS"
> -            CXXFLAGS="$SWR_AVX2_CXXFLAGS $CXXFLAGS"
> -            AC_COMPILE_IFELSE([AC_LANG_PROGRAM()],[],
> -                              [AC_MSG_ERROR([AVX2 compiler support not detected])])
> -            CXXFLAGS="$save_CXXFLAGS"
> -            AC_LANG_POP([C++])
> -
> +            AC_MSG_CHECKING([whether $CXX supports c++11])
> +            if ! swr_cxx_feature_flags_check \
> +                "#if __cplusplus < 201103L" \
> +                ",-std=c++11"; then
> +                AC_MSG_RESULT([no])
> +                AC_MSG_ERROR([swr requires C++11 support])
> +            fi
> +            AC_MSG_RESULT([$SWR_CXX_FEATURE_FLAGS])
> +            CXXFLAGS="$SWR_CXX_FEATURE_FLAGS $CXXFLAGS”

We don’t want to globally override CXXFLAGS; AC_SUBST on a SWR_CXXFLAGS and using that in swr’s Makefile.am would be better.

> +
> +            AC_MSG_CHECKING([whether $CXX supports AVX])
> +            if ! swr_cxx_feature_flags_check \
> +                "#ifndef __AVX__" \
> +                ",-mavx,-march=core-avx"; then
> +                AC_MSG_RESULT([no])
> +                AC_MSG_ERROR([swr requires AVX compiler support])
> +            fi
> +            AC_MSG_RESULT([$SWR_CXX_FEATURE_FLAGS])
> +            SWR_AVX_CXXFLAGS="$SWR_CXX_FEATURE_FLAGS"
>             AC_SUBST([SWR_AVX_CXXFLAGS])
> +
> +            AC_MSG_CHECKING([whether $CXX supports AVX2])
> +            if ! swr_cxx_feature_flags_check \
> +                "#if !(defined(__AVX2__)&&defined(__FMA__)&&defined(__BMI2__)&&defined(__F16C__))” \

Is there any standard that says these are defined if the compiler supports them?  With icc 16.0.3, the test falls into the #error path when it tries the fallback test of -march=core-avx2.

> +                ",-mavx2 -mfma -mbmi2 -mf16c,-march=core-avx2"; then
> +                AC_MSG_RESULT([no])
> +                AC_MSG_ERROR([swr requires AVX2 compiler support])
> +            fi
> +            AC_MSG_RESULT([$SWR_CXX_FEATURE_FLAGS])
> +            SWR_AVX2_CXXFLAGS="$SWR_CXX_FEATURE_FLAGS"
>             AC_SUBST([SWR_AVX2_CXXFLAGS])
> 
>             HAVE_GALLIUM_SWR=yes
> -- 
> 2.5.5
>