[Mesa-dev] [PATCH 1/6] configure.ac: Rename MESA_LLVM to FOUND_LLVM

[Emil Velikov]

On 8 December 2016 at 22:12, Tobias Droste <tdroste at gmx.de> wrote:
> Am Donnerstag, 8. Dezember 2016, 16:59:27 CET schrieb Emil Velikov:
>> On 8 December 2016 at 02:03, Tobias Droste <tdroste at gmx.de> wrote:
>> > this renames MESA_LLVM to FOUND_LLVM and updates the config.log report
>> > to say if LLVM is found or not, to make clear that this does not mean
>> > that it is used.
>> >
>> > Signed-off-by: Tobias Droste <tdroste at gmx.de>
>> > ---
>> >
>> >  configure.ac | 21 ++++++++++-----------
>> >  1 file changed, 10 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/configure.ac b/configure.ac
>> > index adca49d31e..1499380c45 100644
>> > --- a/configure.ac
>> > +++ b/configure.ac
>> > @@ -997,15 +997,15 @@ llvm_set_environment_variables() {
>> >
>> >          fi
>> >
>> >          DEFINES="${DEFINES} -DHAVE_LLVM=0x0$LLVM_VERSION_INT
>> >          -DMESA_LLVM_VERSION_PATCH=$LLVM_VERSION_PATCH">
>> > -        MESA_LLVM=1
>> > +        FOUND_LLVM=yes
>> >
>> >      else
>> >
>> > -        MESA_LLVM=0
>> > +        FOUND_LLVM=no
>> >
>> >          LLVM_VERSION_INT=0
>> >
>> >      fi
>> >
>> >  }
>> >
>> >  llvm_check_version_for() {
>> >
>> > -    if test "x$MESA_LLVM" = x0; then
>> > +    if test "x$FOUND_LLVM" = xno; then
>> >
>> >          AC_MSG_ERROR([LLVM $1 or newer is required for $2])
>> >          return
>> >
>> >      fi
>> >
>> > @@ -1065,7 +1065,6 @@ radeon_llvm_check() {
>> >
>> >  llvm_set_environment_variables
>> >
>> > -AC_SUBST([MESA_LLVM])
>> >
>> >  AC_SUBST([LLVM_BINDIR])
>> >  AC_SUBST([LLVM_CFLAGS])
>> >  AC_SUBST([LLVM_CPPFLAGS])
>> >
>> > @@ -2507,7 +2506,7 @@ if test -n "$with_gallium_drivers"; then
>> >
>> >              ;;
>> >
>> >          xswrast)
>> >
>> >              HAVE_GALLIUM_SOFTPIPE=yes
>> >
>> > -            if test "x$MESA_LLVM" = x1 && test "x$enable_gallium_llvm" ==
>> > "xyes";  then +            if test "x$FOUND_LLVM" = xyes && test
>> > "x$enable_gallium_llvm" == "xyes";  then
>> Something I've noticed a bit too late - using == may work, but is incorrect.
>> Sorry about that :-\
>
> Oh yes, you are right! Will post a v2 as soon as we figured out what to do
> with the other patches.
>
>>
>> Maybe
>> if test "x$FOUND_LLVM" = xyes -a "x$enable_gallium_llvm" = "xyes";  then
>>
>> >                  HAVE_GALLIUM_LLVMPIPE=yes
>> >
>> >              fi
>> >              ;;
>> >
>> > @@ -2566,7 +2565,7 @@ dnl by calling llvm-config --libs
>> > ${DRIVER_LLVM_COMPONENTS}, but>
>> >  dnl this was causing the same libraries to be appear multiple times
>> >  dnl in LLVM_LIBS.
>> >
>> > -if test "x$MESA_LLVM" != x0; then
>> > +if test "x$FOUND_LLVM" != xno; then
>> >
>> >      if ! $LLVM_CONFIG --libs ${LLVM_COMPONENTS} >/dev/null; then
>> >
>> >         AC_MSG_ERROR([Calling ${LLVM_CONFIG} failed])
>> >
>> > @@ -2666,7 +2665,7 @@ AM_CONDITIONAL(NEED_RADEON_DRM_WINSYS, test
>> > "x$HAVE_GALLIUM_R300" = xyes -o \>
>> >  AM_CONDITIONAL(NEED_WINSYS_XLIB, test "x$enable_glx" = xgallium-xlib)
>> >  AM_CONDITIONAL(NEED_RADEON_LLVM, test x$NEED_RADEON_LLVM = xyes)
>> >  AM_CONDITIONAL(HAVE_GALLIUM_COMPUTE, test x$enable_opencl = xyes)
>> >
>> > -AM_CONDITIONAL(HAVE_GALLIUM_LLVM, test "x$MESA_LLVM" = x1 -a \
>> > +AM_CONDITIONAL(HAVE_GALLIUM_LLVM, test "x$FOUND_LLVM" = xyes -a \
>> >
>> >                                         "x$enable_gallium_llvm" = xyes)
>> >
>> >  AM_CONDITIONAL(USE_VC4_SIMULATOR, test x$USE_VC4_SIMULATOR = xyes)
>> >  if test "x$USE_VC4_SIMULATOR" = xyes -a "x$HAVE_GALLIUM_ILO" = xyes; then
>> >
>> > @@ -2948,12 +2947,12 @@ else
>> >
>> >  fi
>> >
>> >  echo ""
>> >
>> > -if test "x$MESA_LLVM" = x1; then
>> > -    echo "        llvm:            yes"
>> > +if test "x$FOUND_LLVM" = xyes; then
>> > +    echo "        llvm found:      yes"
>> >
>> >      echo "        llvm-config:     $LLVM_CONFIG"
>> >      echo "        llvm-version:    $LLVM_VERSION"
>> >
>> >  else
>> >
>> > -    echo "        llvm:            no"
>> > +    echo "        llvm found:      no"
>> >
>> >  fi
>>
>> In hindsight - even if we say "found" here, it might be confusing. If
>> so, the alternative is to either a) track all the places which require
>> LLVM in the above/below guards ro b) have a require_llvm which is set
>> by each consumer.
>
> The tracking is done by patch 2. Every consumer checks for a specific llvm
> version. That's more or less a fool proof way of tracking.
> I could split it out into a separate function but it would be called after/
> before each version check so there's not really something to gain.
>
> After patch 4 you get something like this:
>
>         llvm found:      yes
>         llvm-config:     <somepath>/llvm-config
>         llvm-version:    4.0.0
>         llvm used:       yes
>
>         Gallium drivers: swrast
>         Gallium st:      mesa
>         Gallium llvm:    no
>
>
> This means llvm was found, is used in one or more locations, but is disabled
> for gallium.
>
Idea is that there is only one place (gallivm) that has LLVM as
optional. Either way, I'm testing the patches locally and barring any
issues I'll squash any merge conflicts and push the lot.

Thanks
Emil