[Mesa-dev] [PATCH 6/8] configure.ac: set LLVM_{C, CXX, LD}FLAGS only as needed

[Tobias Droste]

Am Mittwoch, 18. Januar 2017, 13:54:02 CET schrieb Emil Velikov:
> From: Emil Velikov <emil.velikov at collabora.com>
> 
> Earlier refactoring commits started setting the above regardless if LLVM
> is used or not. Move them to the respective section to restore the
> original functionality.
> 
> Since we require the preprocessor flags (includes in particular) for the
> header version parsing keep those as-is. They are not used outside of
> configure.ac thus should not cause any side-effects.
> 
> As-is adding the C/CXXFLAGS can lead to build issues on when
> cross-compiling.
> 
> Cc: Tobias Droste <tdroste at gmx.de>
> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: Tomasz Figa <tfiga at chromium.org>
> Reported-by: Ilia Mirkin <imirkin at alum.mit.edu>
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
> We still want to reworking the C/CPP/CXX sanitazation, but that's
> orthogonal to the issue at hand.
> ---
>  configure.ac | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 1ec8e42882..7580fd955f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -971,16 +971,13 @@ strip_unwanted_llvm_flags() {
>  llvm_set_environment_variables() {
>      if test "x$LLVM_CONFIG" != xno; then
>          LLVM_VERSION=`$LLVM_CONFIG --version | egrep -o '^[[0-9.]]+'`
> -        LLVM_LDFLAGS=`$LLVM_CONFIG --ldflags`
>          LLVM_CPPFLAGS=`strip_unwanted_llvm_flags "$LLVM_CONFIG --cppflags"`
> -        LLVM_CFLAGS=$LLVM_CPPFLAGS   # CPPFLAGS seem to be sufficient -   
>     LLVM_CXXFLAGS=`strip_unwanted_llvm_flags "$LLVM_CONFIG --cxxflags"`
> LLVM_INCLUDEDIR=`$LLVM_CONFIG --includedir`
>          LLVM_LIBDIR=`$LLVM_CONFIG --libdir`
> 
> -        # We need to respect LLVM_CFLAGS when compiling LLVM headers.
> +        # We need to respect LLVM_CPPFLAGS when compiling LLVM headers.
>          save_CFLAGS="$CFLAGS"
> -        CFLAGS="$CFLAGS $LLVM_CFLAGS"
> +        CFLAGS="$CFLAGS $LLVM_CPPFLAGS"

if we say we need LLVM_CPPFLAGS shouldn't we also save/set/restore CPPFLAGS 
instead of CFLAGS?

Not sure how this is implemented and it's probably always using a C compiler 
so this doesn't change the outcome, but it seems weird to set CFLAGS to 
CPPFLAGS :-)

Either way:
Reviewed-by: Tobias Droste <tdroste at gmx.de>

> 
>          AC_COMPUTE_INT([LLVM_VERSION_MAJOR], [LLVM_VERSION_MAJOR],
>              [#include "${LLVM_INCLUDEDIR}/llvm/Config/llvm-config.h"])
> @@ -2584,6 +2581,10 @@ dnl Set HAVE_LLVM compiler define if LLVM is used.
>  if test "x$USE_LLVM" = xyes; then
>      DEFINES="${DEFINES} -DHAVE_LLVM=0x0$LLVM_VERSION_INT
> -DMESA_LLVM_VERSION_PATCH=$LLVM_VERSION_PATCH"
> 
> +    LLVM_LDFLAGS=`$LLVM_CONFIG --ldflags`
> +    LLVM_CFLAGS=$LLVM_CPPFLAGS   # CPPFLAGS seem to be sufficient
> +    LLVM_CXXFLAGS=`strip_unwanted_llvm_flags "$LLVM_CONFIG --cxxflags"`
> +
>      dnl Set LLVM_LIBS - This is done after the driver configuration so
>      dnl that drivers can add additional components to LLVM_COMPONENTS.
>      dnl Previously, gallium drivers were updating LLVM_LIBS directly