[Mesa-dev] [PATCH] swr: bump minimum supported LLVM version to 5.0

[Emil Velikov]

On 19 June 2018 at 10:53, Juan A. Suarez Romero <jasuarez at igalia.com> wrote:
> On Mon, 2018-06-18 at 16:29 +0100, Eric Engestrom wrote:
>> On Monday, 2018-06-18 16:23:41 +0200, Juan A. Suarez Romero wrote:
>> > RADV now requires LLVM 5.0 or greater, and thus we can't build dist
>> > tarball because swr requires LLVM 4.0.
>> >
>> > Let's bump required LLVM to 5.0 in swr too.
>> >
>> > Fixes: f9eb1ef870 ("amd: remove support for LLVM 4.0")
>> > Cc: George Kyriazis <george.kyriazis at intel.com>
>> > Cc: Tim Rowley <timothy.o.rowley at intel.com>
>> > Cc: Emil Velikov <emil.velikov at collabora.com>
>> > Cc: Dylan Baker <dylan at pnwbakers.com>
>> > Cc: Eric Engestrom <eric.engestrom at imgtec.com>
>>
>> s/imgtec/intel/ :)
>> (I moved)
>>
>
> Fixed :)
>
>> > ---
>> >  .travis.yml                         | 12 ++++++------
>> >  configure.ac                        |  7 ++++---
>> >  meson.build                         |  4 +---
>> >  src/gallium/drivers/swr/Makefile.am |  6 +++---
>> >  src/gallium/drivers/swr/SConscript  |  4 ++--
>> >  5 files changed, 16 insertions(+), 17 deletions(-)
>> >
>> > diff --git a/.travis.yml b/.travis.yml
>> > index b1fc7de9587..c9a30fa0ef5 100644
>> > --- a/.travis.yml
>> > +++ b/.travis.yml
>> > @@ -92,7 +92,7 @@ matrix:
>> >          - BUILD=make
>> >          - MAKEFLAGS="-j4"
>> >          - MAKE_CHECK_COMMAND="true"
>> > -        - LLVM_VERSION=4.0
>> > +        - LLVM_VERSION=5.0
>> >          - LLVM_CONFIG="llvm-config-${LLVM_VERSION}"
>> >          - OVERRIDE_CC="gcc-4.8"
>> >          - OVERRIDE_CXX="g++-4.8"
>> > @@ -105,12 +105,12 @@ matrix:
>> >        addons:
>> >          apt:
>> >            sources:
>> > -            - llvm-toolchain-trusty-4.0
>> > +            - llvm-toolchain-trusty-5.0
>> >            packages:
>> >              # LLVM packaging is broken and misses these dependencies
>> >              - libedit-dev
>> >              # From sources above
>> > -            - llvm-4.0-dev
>> > +            - llvm-5.0-dev
>> >              # Common
>> >              - xz-utils
>> >              - x11proto-xf86vidmode-dev
>> > @@ -432,7 +432,7 @@ matrix:
>> >          - BUILD=scons
>> >          - SCONSFLAGS="-j4"
>> >          - SCONS_TARGET="swr=1"
>> > -        - LLVM_VERSION=4.0
>> > +        - LLVM_VERSION=5.0
>> >          - LLVM_CONFIG="llvm-config-${LLVM_VERSION}"
>> >          # Keep it symmetrical to the make build. There's no actual SWR, yet.
>> >          - SCONS_CHECK_COMMAND="true"
>> > @@ -441,13 +441,13 @@ matrix:
>> >        addons:
>> >          apt:
>> >            sources:
>> > -            - llvm-toolchain-trusty-4.0
>> > +            - llvm-toolchain-trusty-5.0
>> >            packages:
>> >              - scons
>> >              # LLVM packaging is broken and misses these dependencies
>> >              - libedit-dev
>> >              # From sources above
>> > -            - llvm-4.0-dev
>> > +            - llvm-5.0-dev
>> >              # Common
>> >              - xz-utils
>> >              - x11proto-xf86vidmode-dev
>> > diff --git a/configure.ac b/configure.ac
>> > index 7a0e4754208..543b6fe061b 100644
>> > --- a/configure.ac
>> > +++ b/configure.ac
>> > @@ -110,7 +110,7 @@ LLVM_REQUIRED_OPENCL=3.9.0
>> >  LLVM_REQUIRED_R600=3.9.0
>> >  LLVM_REQUIRED_RADEONSI=5.0.0
>> >  LLVM_REQUIRED_RADV=5.0.0
>> > -LLVM_REQUIRED_SWR=4.0.0
>> > +LLVM_REQUIRED_SWR=5.0.0
>> >
>> >  dnl Check for progs
>> >  AC_PROG_CPP
>> > @@ -2755,8 +2755,9 @@ if test -n "$with_gallium_drivers"; then
>> >  fi
>> >
>> >  # XXX: Keep in sync with LLVM_REQUIRED_SWR
>> > -AM_CONDITIONAL(SWR_INVALID_LLVM_VERSION, test "x$LLVM_VERSION" != x4.0.0 -a \
>> > -                                              "x$LLVM_VERSION" != x4.0.1)
>> > +AM_CONDITIONAL(SWR_INVALID_LLVM_VERSION, test "x$LLVM_VERSION" != x5.0.0 -a \
>> > +                                              "x$LLVM_VERSION" != x5.0.1 -a \
>> > +                                              "x$LLVM_VERSION" != x5.0.2)
>>
>> That check seems designed to break every time something in mesa changes
>> supported llvm version. Is there a reason for it not to be a simple
>> `>= 4.0` check?
>>
>
> Because gen_builder.hpp is a generated file and it contains information that is
> specific to the LLVM version it originates from. Apparently, this file is
> forward compatible, but not backward, and it is included in dist tarball.
>
> I guess the problem is that if you end up building the distball with, let's say
> LLVM 6.0, then the tarball couldn't be built with LLVM 5.0, due this file
> compatibility. Hence why it "forces" to use the minimum common LLVM version for
> all the drivers.
>
> This was added in commit 5233eaf9ee8 ("automake: add SWR LLVM gen_builder.hpp
> workaround") and modified in commit b39f6d5fc7c ("travis: radeonsi and radv need
> LLVM 4.0").
>
Precisely. Currently the python generators produce some _unused_ code.
So when you try to use LLVM 5.0 based sources with LLVM 4.0, you get
compilation failures.

Ideally, the generator will be fixed to remove the unused code, but
that for another day.

With a confirmation from the SWR devs, the patch is
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

-Emil