[Mesa-dev] [PATCH v4 17/18] configure.ac: Only add default LLVM components if needed

[Tobias Droste]

Am Mittwoch, 19. Oktober 2016, 14:17:53 CEST schrieb Emil Velikov:
> On 16 October 2016 at 01:20, Tobias Droste <tdroste at gmx.de> wrote:
> > LLVM components are only added when LLVM is needed.
> > This means gallium adds this as soon as "--enable-gallium-llvm"
> > is "yes" and radv + opencl add it explicitly.
> > 
> > This slightly changes the "auto" value of "--enable-gallium-llvm":
> > If no LLVM is installed, it is set to "no". If later drivers need
> > this they get the error message that "--enable-gallium-llvm" is
> > needed.
> > This way if someone is only building softpipe he doesn't need to
> > specify "--disable-gallium-llvm" and doesn't have to have LLVM
> > installed.
> > 
> > Signed-off-by: Tobias Droste <tdroste at gmx.de>
> > ---
> > 
> >  configure.ac | 21 +++++++++++++++++----
> >  1 file changed, 17 insertions(+), 4 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index bbc1cb5..9e7130ad 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -897,7 +897,11 @@ llvm_add_component() {
> > 
> >      new_llvm_component=$1
> >      driver_name=$2
> > 
> > -    LLVM_COMPONENTS="$LLVM_COMPONENTS $new_llvm_component"
> > +    if $LLVM_CONFIG --components | grep -iqw $new_llvm_component ; then
> > +        LLVM_COMPONENTS="${LLVM_COMPONENTS} ${new_llvm_component}"
> > +    else
> > +        AC_MSG_ERROR([LLVM component '$new_llvm_component' not enabled in
> > your LLVM build. Required by $driver_name.]) +    fi
> > 
> >  }
> >  
> >  llvm_add_default_components() {
> > 
> > @@ -980,8 +984,6 @@ llvm_set_environment_variables() {
> > 
> >              LLVM_VERSION_INT=`echo $LLVM_VERSION | sed -e
> >              's/\([[0-9]]\)\.\([[0-9]]\)/\10\2/g'`>          
> >          fi
> > 
> > -        llvm_add_default_components "gallium"
> > -
> > 
> >          DEFINES="${DEFINES} -DHAVE_LLVM=0x0$LLVM_VERSION_INT
> >          -DMESA_LLVM_VERSION_PATCH=$LLVM_VERSION_PATCH" MESA_LLVM=1
> >      
> >      else
> > 
> > @@ -1007,6 +1009,7 @@ radeon_llvm_check() {
> > 
> >      llvm_add_target $amdgpu_llvm_target_name $4
> > 
> > +    llvm_add_default_components $4
> > 
> >      llvm_add_component "bitreader" $4
> >      llvm_add_component "ipo" $4
> > 
> > @@ -2175,6 +2178,7 @@ if test "x$enable_opencl" = xyes; then
> > 
> >      llvm_check_version_for "3" "6" "0" "opencl"
> > 
> > +    llvm_add_default_components "opencl"
> > 
> >      llvm_add_component "all-targets" "opencl"
> >      llvm_add_component "linker" "opencl"
> >      llvm_add_component "instrumentation" "opencl"
> > 
> > @@ -2322,9 +2326,18 @@ if test -z "$with_gallium_drivers"; then
> > 
> >  fi
> >  if test "x$enable_gallium_llvm" = xauto; then
> >  
> >      case "$host_cpu" in
> > 
> > -    i*86|x86_64|amd64) enable_gallium_llvm=yes;;
> > +    i*86|x86_64|amd64)
> > +        if test "x$MESA_LLVM" = "x1"; then
> > +            enable_gallium_llvm=yes
> > +        else
> > +            enable_gallium_llvm=no
> > +        fi
> 
> This won't work as expected I'm afraid.
> 
> For example: if we've failed during the autodetection of llvm,
> MESA_LLVM will be 0, thus instead of enabling enable_gallium_llvm [on
> supported platforms] and error-ing out you'll get a silent disable.
> The latter of which is not what the llvmpipe/r300 folk would
> appreciate.

If you only build gallium swrast you would get softpipe instead of llvmpipe, 
that's true and intended. 
I think it's better to build softpipe instead of erroring out. If the user 
specifies --enable-gallium-llvm to make sure he gets llvmpipe this hunk 
doesn't do anything.

r300 would actually fail with the message that --enable-gallium-llvm is 
missing.
You can easily test this by running:
LLVM_CONFIG=no ./configure ....

Maybe adjust to the help message to "on x86 if llvm is installed"?

> 
> -Emil