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

[Emil Velikov]

Hi Chuck,

On 28 June 2016 at 20:50, 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 part should have been a separate patch. Please try to keep things
separate for the future.

> This addresses a bug where the Intel compiler will silently ignore the
> AVX2 instruction feature flags and then potentially fail to build.
>
> v2: Pass preprocessor-check argument as true-state instead of
>     false-state for clarity.
> v3: Reduce AVX2 define test to just __AVX2__.  Additional defines suchas
>     __FMA__, __BMI2__, and __F16C__ appear to be inconsistently defined
>     w.r.t thier availability.
> v4: Fix C++11 flags being added globally and add more logic to
>     swr_require_cxx_feature_flags
>
> Cc: Tim Rowley <timothy.o.rowley at intel.com>
> Signed-off-by: Chuck Atkins <chuck.atkins at kitware.com>
> ---
>  configure.ac                        | 73 +++++++++++++++++++++++++------------
>  src/gallium/drivers/swr/Makefile.am |  4 +-
>  2 files changed, 52 insertions(+), 25 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index cc9bc47..8321e8e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2330,6 +2330,45 @@ swr_llvm_check() {
>      fi
>  }
>
> +swr_require_cxx_feature_flags() {
> +    feature_name="$1"
> +    preprocessor_test="$2"
> +    option_list="$3"
> +    output_var="$4"
> +
> +    AC_MSG_CHECKING([whether $CXX supports $feature_name])
> +    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(
> +                [   #if !($preprocessor_test)
> +                    #error
> +                    #endif
> +                ])],
> +            [found=1; break],
> +            [])
> +        IFS=","
> +    done
> +    IFS="$save_IFS"
> +    CXXFLAGS="$save_CXXFLAGS"
> +    AC_LANG_POP([C++])
> +    if test $found -eq 1; then
> +        AC_MSG_RESULT([$opts])
> +        eval "$output_var=\$opts"
> +        return 0
> +    fi
> +    AC_MSG_RESULT([no])
> +    AC_MSG_ERROR([swr requires $feature_name support])
> +    return 1
Esp with the IFS override this looks rather nasty imho.

Just a suggestion: An approach similar to LIBDRM_CC_TRY_FLAG in [1] is
a lot more readable imho.

-Emil
[1] https://cgit.freedesktop.org/mesa/drm/tree/configure.ac