[Mesa-dev] [PATCH] swr: bump minimum supported LLVM version to 5.0
Juan A. Suarez Romero
jasuarez at igalia.com
Tue Jun 19 09:53:12 UTC 2018
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").
J.A.
> That's the hunk that was causing issues, wasn't it? Everything else in
> this patch is just to keep the same version number everywhere?
>
> Other than that, the patch itself looks reasonable, so given a good
> answer to "why is this so fragile?", you can have my:
> Reviewed-by: Eric Engestrom <eric.engestrom at intel.com>
>
> Otherwise, I would prefer to just turn the autotools check into
> a `>= 4.0` like everything else does.
>
> >
> > if test "x$enable_llvm" = "xyes" -a "$with_gallium_drivers"; then
> > llvm_require_version $LLVM_REQUIRED_GALLIUM "gallium"
> > diff --git a/meson.build b/meson.build
> > index 65ae32172d2..a5662160d66 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1130,10 +1130,8 @@ if with_gallium_opencl
> > llvm_optional_modules += ['coroutines', 'opencl']
> > endif
> >
> > -if with_amd_vk or with_gallium_radeonsi
> > +if with_amd_vk or with_gallium_radeonsi or with_gallium_swr
> > _llvm_version = '>= 5.0.0'
> > -elif with_gallium_swr
> > - _llvm_version = '>= 4.0.0'
> > elif with_gallium_opencl or with_gallium_r600
> > _llvm_version = '>= 3.9.0'
> > else
> > diff --git a/src/gallium/drivers/swr/Makefile.am b/src/gallium/drivers/swr/Makefile.am
> > index 8b3150288e6..5cc3f77478a 100644
> > --- a/src/gallium/drivers/swr/Makefile.am
> > +++ b/src/gallium/drivers/swr/Makefile.am
> > @@ -374,9 +374,9 @@ include $(top_srcdir)/install-gallium-links.mk
> > # created with the oldest supported version of LLVM.
> > dist-hook:
> > if SWR_INVALID_LLVM_VERSION
> > - @echo "*******************************************************"
> > - @echo "LLVM 4.0.0 or LLVM 4.0.1 required to create the tarball"
> > - @echo "*******************************************************"
> > + @echo "*****************************************"
> > + @echo "LLVM 5.0.x required to create the tarball"
> > + @echo "*****************************************"
> > @test
> > endif
> >
> > diff --git a/src/gallium/drivers/swr/SConscript b/src/gallium/drivers/swr/SConscript
> > index 528cfac39f6..224372eb3f5 100644
> > --- a/src/gallium/drivers/swr/SConscript
> > +++ b/src/gallium/drivers/swr/SConscript
> > @@ -12,8 +12,8 @@ if not env['llvm']:
> > env['swr'] = False
> > Return()
> >
> > -if env['LLVM_VERSION'] < distutils.version.LooseVersion('4.0'):
> > - print("warning: swr requires LLVM >= 4.0: not building swr")
> > +if env['LLVM_VERSION'] < distutils.version.LooseVersion('5.0'):
> > + print("warning: swr requires LLVM >= 5.0: not building swr")
> > env['swr'] = False
> > Return()
> >
> > --
> > 2.17.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
More information about the mesa-dev
mailing list