On 19 October 2016 at 18:55, Tobias Droste <tdroste at gmx.de> wrote:
> 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.
>
Atm, enable_gallium_llvm will default to yes on x86 even if you don't
have LLVM available. Admittedly one can call this heuristics
"sub-optimal" or worse, but regardless this is not something we should
address here. This patch will change the behaviour and considering the
serious objections* on the topic (esp. on the r300 side) things will
end up being nasty.
*Devs are/were concerned about user miss-configuration leading to "OMG
this sh*t is dead slow" reports.
Thanks
Emil