Bug 99010

9d14a25bee0f1457a82f3e42b3baf3db1806faea is the first bad commit
commit 9d14a25bee0f1457a82f3e42b3baf3db1806faea
Author: Tobias Droste <tdroste@gmx.de>
Date:   Sat Nov 19 02:39:04 2016 +0100

    configure.ac: Move llvm_set_environment_variables higher.
    
    This moves the function to get the LLVM environment variables higher
    in the file. It still needs to be below the "--enable-opencl" because
    it uses $enable_opencl.
    It can be called without condition now as it only throws errors if
    openCL is enabled.
    
    v5:
    HAVE_MESA_LLVM is only used for gallium. Rename it to HAVE_GALLIUM_LLVM.
    In order to only link LLVM when it is needed, HAVE_GALLIUM_LLVM is only
    set if "$enable-gallium-llvm" is yes.
    
    Signed-off-by: Tobias Droste <tdroste@gmx.de>
    Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

:100644 100644 1de6b7e905d33dfa4c69aa2f188db890aba23354 3e9972c7e3204f64bcdc3a4046954e64d5d3cb48 M	configure.ac
:040000 040000 0aff1bbf50001090806ed625865f42b97ba789a8 8812620b919a6a0c64118896ebe124c199a61685 M	src
bisect run success

Hi Vinson,

could you tell me what configure options you're using?

Thanks,
Tobias

(In reply to Tobias Droste from comment #2)
> 
> could you tell me what configure options you're using?
> 

./autogen.sh --disable-gallium-llvm --with-gallium-drivers=swrast

Seems to work fine here. Odd.

It builds a lot of non gallium drivers and src/gallium/drivers/softpipe without any errors.

What error do you see?

Does 'git clean -fdx' fix it? Maybe a stale Makefile.

(In reply to Tobias Droste from comment #4)
> 
> What error do you see?

It builds with llvm.

What component? Auxiliary?

If I look at src/gallium/auxiliary/Makefile I see this:

#am__append_3 = \
#	$(LLVM_CFLAGS)

#am__append_4 = \
#	$(GALLIUM_CFLAGS) \
#	$(LLVM_CXXFLAGS)

#am__append_5 = \
#	$(GALLIVM_SOURCES)

Is it the same for you?

(In reply to Tobias Droste from comment #7)
> What component? Auxiliary?
> 
> If I look at src/gallium/auxiliary/Makefile I see this:
> 
> #am__append_3 = \
> #	$(LLVM_CFLAGS)
> 
> #am__append_4 = \
> #	$(GALLIUM_CFLAGS) \
> #	$(LLVM_CXXFLAGS)
> 
> #am__append_5 = \
> #	$(GALLIVM_SOURCES)
> 
> Is it the same for you?

It is always picking up llvm. On recent distros the configure report shows "llvm: yes". On distros with older llvm, configure fails with "configure: error: Could not find llvm shared libraries:".

Ok now I get it.

It says "llvm: yes" as soon as it finds LLVM that does not automatically mean it is used by something. So this works as expected. 
Maybe the output can be improved.

What LLVM version generates error?

(In reply to Tobias Droste from comment #9)
> Ok now I get it.
> 
> It says "llvm: yes" as soon as it finds LLVM that does not automatically
> mean it is used by something. So this works as expected. 
> Maybe the output can be improved.
> 
> What LLVM version generates error?

Ubuntu 12.04 llvm-2.9

Ok, I take a look at this tomorrow (it's late here). 
Sorry for the problems.

For a quick workaround:
Set the env variable LLVM_CONFIG to "no".

e.g.: LLVM_CONFIG=no ./autogen.sh ...

(In reply to Vinson Lee from comment #10)

> Ubuntu 12.04 llvm-2.9

Afaict nothing in Mesa supports such an old version of LLVM. So I'm not sure how much we should worry about here. Either way, some things that come to mind:

 - re the above issue:
Move the base LLVM version validation to llvm_set_environment_variables

 - re the misleading LLVM message
1) Rename MESA_LLVM to FOUND_LLVM or alike.
2) Drop the unused AC_SUBST([MESA_LLVM])
3) During the output/summary stage provide Found/Detected and Using sections ?

Related:
- gallium_require_llvm and llvm_check_version_for are wrapped too confusingly
Some callers of the former also call the latter

I also get this error when trying to use softpipe.

libGL: dlopen lib/gallium/swrast_dri.so failed (lib/gallium/swrast_dri.so: undefined symbol: draw_gs_llvm_destroy_variant)

Oops.

Fixes sent to mailing list:
https://lists.freedesktop.org/archives/mesa-dev/2016-December/137798.html

You now get a output like this:

        llvm found:      yes
        llvm-config:     <somepath>/llvm-config
        llvm-version:    4.0.0
        llvm used:       yes

        Gallium drivers: swrast
        Gallium st:      mesa
        Gallium llvm:    no

with the important part being "Gallium llvm:    no"

Tobias,

Any update for your patch series on the mailing list? Looks like there's a few comments for other developers requiring you to respin a version 2 before it will land in Mesa master.

I'm affected by this regression since your commit of:

   commit 9d14a25bee0f1457a82f3e42b3baf3db1806faea
   Author: Tobias Droste <tdroste@gmx.de>
   Date:   Sat Nov 19 02:39:04 2016 +0100

Using llvm 3.5, on Travis-CI, to do regression testing of xserver.

(In reply to Rhys Kidd from comment #15)
> Tobias,
> 
> Any update for your patch series on the mailing list? Looks like there's a
> few comments for other developers requiring you to respin a version 2 before
> it will land in Mesa master.
> 
> I'm affected by this regression since your commit of:
> 
>    commit 9d14a25bee0f1457a82f3e42b3baf3db1806faea
>    Author: Tobias Droste <tdroste@gmx.de>
>    Date:   Sat Nov 19 02:39:04 2016 +0100
> 
> Using llvm 3.5, on Travis-CI, to do regression testing of xserver.

Hey Rhys,

sorry haven't heard back from Emil yet about what to do next. 
Christmas and new year aren't the best times to get a lot of time to do work :-) 
(Or did I miss it?)

My last mail: 
https://lists.freedesktop.org/archives/mesa-dev/2016-December/138436.html

But even if there's a V2 it will mostly be cosmetics. This version at least solves the problem, so it doesn't make things worse for you if you apply them yourself on your server until this or another version will be merged.

(In reply to Tobias Droste from comment #11)
> Ok, I take a look at this tomorrow (it's late here). 
> Sorry for the problems.
> 
> For a quick workaround:
> Set the env variable LLVM_CONFIG to "no".
> 
> e.g.: LLVM_CONFIG=no ./autogen.sh ...

By the way, if you don't want to apply patches on your server, this workaround should help you too.

(In reply to Tobias Droste from comment #17)
> (In reply to Tobias Droste from comment #11)
> > Ok, I take a look at this tomorrow (it's late here). 
> > Sorry for the problems.
> > 
> > For a quick workaround:
> > Set the env variable LLVM_CONFIG to "no".
> > 
> > e.g.: LLVM_CONFIG=no ./autogen.sh ...
> 
> By the way, if you don't want to apply patches on your server, this
> workaround should help you too.

Yes, that is what I'm using for now until the patch series lands.

Gents do give this series a try

https://patchwork.freedesktop.org/series/19269/

(In reply to Emil Velikov from comment #19)
> Gents do give this series a try
> 
> https://patchwork.freedesktop.org/series/19269/

With the patch series I can build Mesa without setting LLVM_CONFIG=no in the environment again on OpenBSD/amd64.

To prevent Mesa from using llvm if found I still have to use
--disable-gallium-llvm --disable-llvm-shared-libs

Building with just --disable-gallium-llvm gives

configure: error: Could not find llvm shared libraries:
        Please make sure you have built llvm with the --enable-shared option
        and that your llvm libraries are installed in /usr/local/lib
        If you have installed your llvm libraries to a different directory you
        can use the --with-llvm-prefix= configure flag to specify this directory.
        NOTE: Mesa is attempting to use llvm shared libraries by default.
        If you do not want to build with llvm shared libraries and instead want to
        use llvm static libraries then add --disable-llvm-shared-libs to your configure
        invocation and rebuild.

running autogen/configure with:

export AUTOMAKE_VERSION=1.12
export AUTOCONF_VERSION=2.69
export ACLOCAL="aclocal -I /usr/X11R6/share/aclocal"
export PKG_CONFIG_PATH=/usr/X11R6/lib/pkgconfig
export X11BASE=/usr/X11R6
if [[ $(uname -p) == "i386" ]]; then
        export USER_CFLAGS="-march=i586"
        export USER_CXXFLAGS="-march=i586"
fi
./autogen.sh \
--with-gallium-drivers=r300,r600,swrast \
--with-dri-drivers=i915,i965,r200,radeon,swrast \
--disable-silent-rules \
--disable-gallium-llvm \
--disable-llvm-shared-libs \
--disable-glx-tls \
--enable-gles1 --enable-gles2 \
--enable-shared-glapi \
--enable-osmesa \
--enable-debug \
--enable-gbm \
--enable-texture-float \
--with-egl-platforms="x11,drm" \
--prefix=${X11BASE} \
--with-dri-driverdir=${X11BASE}/lib/modules/dri \
--with-dri-searchpath=${X11BASE}/lib/modules/dri

This is with some additional patches, including patching out the requirement of llvm for r300 to mimic building on a system with no llvm.

The OpenBSD ports LLVM package installs static libraries for each component, not a shared library for each or a single shared library for everything which seems to be the approach taken by linux distributions?

Created attachment 129410 [details] [review]
Patch on top of Emils series

Does the attached patch fix this?

Does this also happen if you use --disable-llvm instead of --disable-gallium-llvm?

(In reply to Jonathan Gray from comment #20)
> (In reply to Emil Velikov from comment #19)
> > Gents do give this series a try
> > 
> > https://patchwork.freedesktop.org/series/19269/
> 
> With the patch series I can build Mesa without setting LLVM_CONFIG=no in the
> environment again on OpenBSD/amd64.
> 
Nice, thanks. I'm leaning that the rest of the problems are related to the last two patches in the series - please revert and if so, let's keep that separate from this report.

> To prevent Mesa from using llvm if found I still have to use
> --disable-gallium-llvm --disable-llvm-shared-libs
> 

I cannot see reproduce the issue (nor see how it should happen) with or w/o the --disable-llvm-shared-libs. I've used exact same autogen.sh barring the r300 and X11BASE lines.

I'm leaning that it's OpenBSD specific and likely related to https://github.com/jonathangray/mesa/commit/ce576d7773e9168ec9aca657115c9ba0ec959315

Speaking of which - did you have the change to test it across multiple LLVM versions (and ideally a Linux distro or two) ?

(In reply to Tobias Droste from comment #21)
> Created attachment 129410 [details] [review] [review]
> Patch on top of Emils series
> 
> Does the attached patch fix this?
> 
Afaics the patch is off --enable-gallium-llvm should _not_ override --enable-llvm, but the other way around.

> Does this also happen if you use --disable-llvm instead of
> --disable-gallium-llvm?

Perhaps enable_llvm=auto [from AC_ARG_ENABLE([llvm]...) is overriding the existing enable_llvm value ? Afaict that should _not_ happen (and according to my configure it doesn't) although your suggestion will prove us right/wrong.

Either case: let's first establish if the --enable-llvm patch is to blame, and if so keep move this elsewhere.

(In reply to Emil Velikov from comment #22)
> (In reply to Jonathan Gray from comment #20)
> > (In reply to Emil Velikov from comment #19)
> > > Gents do give this series a try
> > > 
> > > https://patchwork.freedesktop.org/series/19269/
> > 
> > With the patch series I can build Mesa without setting LLVM_CONFIG=no in the
> > environment again on OpenBSD/amd64.
> > 
> Nice, thanks. I'm leaning that the rest of the problems are related to the
> last two patches in the series - please revert and if so, let's keep that
> separate from this report.
> 
> > To prevent Mesa from using llvm if found I still have to use
> > --disable-gallium-llvm --disable-llvm-shared-libs
> > 
> 
> I cannot see reproduce the issue (nor see how it should happen) with or w/o
> the --disable-llvm-shared-libs. I've used exact same autogen.sh barring the
> r300 and X11BASE lines.

I suspect you'll have to install llvm without enabling LLVM_BUILD_LLVM_DYLIB (which defaults to False) so that libLLVM.so is not installed.

> 
> I'm leaning that it's OpenBSD specific and likely related to
> https://github.com/jonathangray/mesa/commit/
> ce576d7773e9168ec9aca657115c9ba0ec959315
> 
> Speaking of which - did you have the change to test it across multiple LLVM
> versions (and ideally a Linux distro or two) ?

I largely stopped testing linking against LLVM around the time they started requiring the latest c++ standard for their headers.

Using just --disable-llvm works and just --disable-gallium-llvm works as well, so it looks like I got confused in testing earlier today, apologies.

(In reply to Emil Velikov from comment #23)
> (In reply to Tobias Droste from comment #21)
> > Created attachment 129410 [details] [review] [review] [review]
> > Patch on top of Emils series
> > 
> > Does the attached patch fix this?
> > 
> Afaics the patch is off --enable-gallium-llvm should _not_ override
> --enable-llvm, but the other way around.
> 
> > Does this also happen if you use --disable-llvm instead of
> > --disable-gallium-llvm?
> 
> Perhaps enable_llvm=auto [from AC_ARG_ENABLE([llvm]...) is overriding the
> existing enable_llvm value ? Afaict that should _not_ happen (and according
> to my configure it doesn't) although your suggestion will prove us
> right/wrong.

Yes, that's what I was thinking and wanted to try with the patch.

> 
> Either case: let's first establish if the --enable-llvm patch is to blame,
> and if so keep move this elsewhere.

But since it's working now, there's no need to test the patch :-)

This [and a few other] issue[s] should now be resolved.

Ensure that you're using bdd6147e29eb96645b7bf9bb50e9ccc252512be3 or later checkout.