[Mesa-dev] [PATCH v3 25/25] configure.ac: Add required LLVM versions to the top

[Emil Velikov]

On 12 October 2016 at 19:37, Tobias Droste <tdroste at gmx.de> wrote:
> Am Mittwoch, 12. Oktober 2016, 11:53:50 CEST schrieb Emil Velikov:
>> >
>> > +dnl LLVM versions
>> > +LLVM_VERSION_REQUIRED_GALLIUM=3.3.0
>> > +LLVM_VERSION_REQUIRED_LLVMPIPE=3.6.0
>>
>> Not exactly sure why/how llvmpipe got 3.6 since the driver reuses the
>> "gallium" one.
>
> Yes you're right. I'm going to change this to 3.3.0.
>
>>
>> > +LLVM_VERSION_REQUIRED_OPENCL=3.6.0
>> > +LLVM_VERSION_REQUIRED_R600=3.6.0
>> > +LLVM_VERSION_REQUIRED_RADEONSI=3.6.0
>>
>> There's a small related gotcha: as-is at build time we get the
>> different codepaths thus, as people build against shared LLVM (hello
>> Archlinux, I'm looking at you) and update their LLVM without
>> rebuilding mesa (Arch I'm looking at you again) things go funny.
>>
>> Tl;Dr; We really want to enable static linking by default and prod
>> distros to use it.
>
> I'm all in favor of statically linking LLVM (that's the way I'm doing this on
> my pc).
> I think the only reason this is not done is because people (also here on the
> list) don't want any static linkg of external libraries because of size or
> whatever.
> So changing the default to static is easy, but I doubt it will make everyone
> happy ;-)
>
They still have the tools to shoot themselves if they so desire ;-)

>>
>> As an alternative (or in parallel really) we could bump to 3.6.2...
>>
>> > +LLVM_VERSION_REQUIRED_RADV=3.9.0
>> > +LLVM_VERSION_REQUIRED_SWR=3.6.0
>>
>> ... and 3.6.1 + drop the older codepaths. Just a follow-up idea.
>>
>> > +
>>
>> Nit: Drop _VERSION part ?
>
> Ok.
>
>>
>> Follow-up: worth checking with VMWare guys (Jose et al.) it they're
>> still using pre-3.6 llvm and doing code/configure cleanups.
>>
>> >  dnl Check for progs
>> >  AC_PROG_CPP
>> >  AC_PROG_CC
>> >
>> > @@ -935,29 +944,38 @@ llvm_get_version() {
>> >
>> >              [#include "${LLVM_INCLUDEDIR}/llvm/Config/llvm-config.h"])
>> >
>> >          AC_COMPUTE_INT([LLVM_VERSION_MINOR], [LLVM_VERSION_MINOR],
>> >
>> >              [#include "${LLVM_INCLUDEDIR}/llvm/Config/llvm-config.h"])
>> >
>> > +        AC_COMPUTE_INT([LLVM_VERSION_PATCH], [LLVM_VERSION_PATCH],
>> > +            [#include "${LLVM_INCLUDEDIR}/llvm/Config/llvm-config.h"])
>> > +
>> > +        if test -z "$LLVM_VERSION_PATCH"; then
>> > +            LLVM_VERSION_PATCH=`echo $LLVM_VERSION | cut -d. -f3 | egrep
>> > -o '^[[0-9]]+'` +        fi
>>
>> IIRC the LLVM_VERSION parsing was added by the AMD folks since old
>> versions did not have the define. Might be worth checking with them
>> (and llvm headers/codebase) if that's no longer the case given their
>> min. required version.
>>
>> Just an idea, in parallel to the above "require !=0 minor for swr/radeonsi"
>
> 3.6.0 has this already, so if only amd needs this, I'm going to drop this.
>
IIRC r600/radeonsi and swr required the patch version, but let's keep
to a later stage after checking with the devs.

>> .
>> > -        LLVM_VERSION_PATCH=`echo $LLVM_VERSION | cut -d. -f3 | egrep -o
>> > '^[[0-9]]+'`>
>> >          if test -z "$LLVM_VERSION_PATCH"; then
>> >
>> >              LLVM_VERSION_PATCH=0
>> >
>> >          fi
>> >
>> >          if test -n "${LLVM_VERSION_MAJOR}"; then
>> >
>> > -
>> > LLVM_VERSION_INT="${LLVM_VERSION_MAJOR}0${LLVM_VERSION_MINOR}"
>> > -        else
>> > -            LLVM_VERSION_INT=`echo $LLVM_VERSION | sed -e
>> > 's/\([[0-9]]\)\.\([[0-9]]\)/\10\2/g'` +
>> > LLVM_VERSION="${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_
>> > PATCH}">
>> >          fi
>>
>> Follow-up idea: just bail out if the LLVM_VERSION_* defines are not
>> set, and reuse those to construct HAVE_LLVM amongst others (dropping
>> the sed 'magic') or use them all together in favour of our local
>> macros ?
>
> The sed magic is needed there to add the leading 0 to the minor version if
> it's < 10:
>
> 3.6.1           -> 3061
> 3.10.2          -> 3102
>
> If I can always use the header provided values this could be done easily with
> some if < 9 then ... else ... fi
>
>>
>> > -        DEFINES="${DEFINES} -DHAVE_LLVM=0x0$LLVM_VERSION_INT
>> > -DMESA_LLVM_VERSION_PATCH=$LLVM_VERSION_PATCH" +
>> > LLVM_VERSION_INT=`echo $LLVM_VERSION | sed -e
>> > 's/\.\([[1-9]][[0-9]]\)\.\([[0-9]]\)/\1\2/' -e
>> > 's/\.\([[0-9]]\)\.\([[0-9]]\)/0\1\2/' -e 's/\.\([[1-9]][[0-9]]\)/\10/' -e
>> > 's/\.\([[0-9]]\)/0\10/'` +        HAVE_LLVM=`echo $LLVM_VERSION | sed -e
>> > 's/\.\([[1-9]][[0-9]]\)\.\([[0-9]]\)/\1/' -e
>> > 's/\.\([[0-9]]\)\.\([[0-9]]\)/0\1/' -e 's/\.\([[1-9]][[0-9]]\)/\1/' -e
>> > 's/\.\([[0-9]]\)/0\1/'` +
>>
>> The sed patterns have changed, please elaborate [a bit] why/how in the
>> commit msg.
>
> This just makes the conversion for double digit minor numbers work. It didn't
> work with the old script.
>
>>
>> >  llvm_check_version_for() {
>> >
>> > -    if test "${LLVM_VERSION_INT}${LLVM_VERSION_PATCH}" -lt
>> > "${1}0${2}${3}"; then -        AC_MSG_ERROR([LLVM $1.$2.$3 or newer is
>> > required for $4]) +    llvm_target_version=`echo $1 | sed -e
>> > 's/\.\([[1-9]][[0-9]]\)\.\([[0-9]]\)/\1\2/' -e
>> > 's/\.\([[0-9]]\)\.\([[0-9]]\)/0\1\2/' -e 's/\.\([[1-9]][[0-9]]\)/\10/' -e
>> > 's/\.\([[0-9]]\)/0\10/'` +
>>
>> A something as simple as the following should do it as well, shouldn't it ?
>>   llvm_target_version=`echo $1 | sed 's/\.//g'`
>
> Sadly no, because of the leading zero in case there are double digit minor
> numbers.
>
Here and above - there are no double digit minors, yet that we support
so I'd leave that to a later stage.

-Emil