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

Marek Olšák maraeo at gmail.com
Tue Jun 26 01:49:21 UTC 2018


Good timing. I have a patch that removes LLVM 5.0 support from AMD
Mesa drivers. :)

Marek

On Thu, Jun 21, 2018 at 5:51 AM, Eric Engestrom
<eric.engestrom at intel.com> wrote:
> On Thursday, 2018-06-21 11:42:29 +0200, Juan A. Suarez Romero wrote:
>> On Tue, 2018-06-19 at 11:53 +0200, Juan A. Suarez Romero 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").
>> >
>> >
>> >     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>
>> > >
>>
>>
>> Eric, Is the answer for "why is this so fragile?" enough for you? :)
>
> Yep, sorry I forgot to reply, but my r-b does stand :)
>
> Like you and Emil both said, this should be fixed at some point, but for now
> this is how things are.
>
>>
>>
>> > > 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
>> > >
>> > >
>> >
>> > _______________________________________________
>> > mesa-dev mailing list
>> > mesa-dev at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> 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