Bug 151535

Summary: [GTK] update-webkitgtk-libs cannot build mesa
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, clopez, commit-queue, gns, mcatanzaro, mrobinson, ossy
Priority: P2    
Version: Other   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Michael Catanzaro 2015-11-21 15:40:02 PST
The problem is an LLVM API change. It works on the bots because the bots have an old version of LLVM, and mesa in the jhbuild moduleset is missing a dependency on LLVM. We need to switch to a newer version of mesa that's compatible with LLVM 3.7, or else downgrade LLVM (but that would presumably break FTL, so let's upgrade mesa). Regardless, we have to build LLVM before mesa.


/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Source/Mesa/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp: In function ‘size_t disassemble(const void*, llvm::raw_ostream&)’:
/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Source/Mesa/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp:260:78: error: no matching function for call to ‘llvm::Target::createMCInstPrinter(unsigned int&, const llvm::MCAsmInfo&, const llvm::MCInstrInfo&, const llvm::MCRegisterInfo&, const llvm::MCSubtargetInfo&) const’
          T->createMCInstPrinter(AsmPrinterVariant, *AsmInfo, *MII, *MRI, *STI));
                                                                              ^
In file included from /home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Source/Mesa/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp:42:0:
/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Root/include/llvm/Support/TargetRegistry.h:410:18: note: candidate: llvm::MCInstPrinter* llvm::Target::createMCInstPrinter(const llvm::Triple&, unsigned int, const llvm::MCAsmInfo&, const llvm::MCInstrInfo&, const llvm::MCRegisterInfo&) const
   MCInstPrinter *createMCInstPrinter(const Triple &T, unsigned SyntaxVariant,
                  ^
/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Root/include/llvm/Support/TargetRegistry.h:410:18: note:   no known conversion for argument 1 from ‘unsigned int’ to ‘const llvm::Triple&’
/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Source/Mesa/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp:280:54: error: no matching function for call to ‘llvm::TargetMachine::getSubtargetImpl()’
    const TargetInstrInfo *TII = TM->getSubtargetImpl()->getInstrInfo();
                                                      ^
In file included from /home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Source/Mesa/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp:31:0:
/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Root/include/llvm/Target/TargetMachine.h:114:38: note: candidate: virtual const llvm::TargetSubtargetInfo* llvm::TargetMachine::getSubtargetImpl(const llvm::Function&) const
   virtual const TargetSubtargetInfo *getSubtargetImpl(const Function &) const {
                                      ^
/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Root/include/llvm/Target/TargetMachine.h:114:38: note:   candidate expects 1 argument, 0 provided
/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Source/Mesa/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp:331:40: error: no matching function for call to ‘llvm::MCInstPrinter::printInst(llvm::MCInst*, llvm::raw_ostream&, const char [1])’
       Printer->printInst(&Inst, Out, "");
                                        ^
In file included from /home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Source/Mesa/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp:52:0:
/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Root/include/llvm/MC/MCInstPrinter.h:72:16: note: candidate: virtual void llvm::MCInstPrinter::printInst(const llvm::MCInst*, llvm::raw_ostream&, llvm::StringRef, const llvm::MCSubtargetInfo&)
   virtual void printInst(const MCInst *MI, raw_ostream &OS, StringRef Annot,
                ^
/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Root/include/llvm/MC/MCInstPrinter.h:72:16: note:   candidate expects 4 arguments, 3 provided
Makefile:2221: recipe for target 'gallivm/lp_bld_debug.lo' failed
make[4]: *** [gallivm/lp_bld_debug.lo] Error 1
make[4]: *** Waiting for unfinished jobs....
ar: `u' modifier ignored since `D' is the default (see `U')
/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Source/Mesa/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp: In function ‘LLVMBool lp_build_create_jit_compiler_for_module(LLVMOpaqueExecutionEngine**, lp_generated_code**, LLVMModuleRef, LLVMMCJITMemoryManagerRef, unsigned int, int, char**)’:
/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Source/Mesa/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp:433:12: error: ‘class llvm::TargetOptions’ has no member named ‘JITEmitDebugInfo’
    options.JITEmitDebugInfo = true;
            ^
/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Source/Mesa/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp:442:12: error: ‘class llvm::TargetOptions’ has no member named ‘NoFramePointerElim’
    options.NoFramePointerElim = true;
            ^
Makefile:2221: recipe for target 'gallivm/lp_bld_misc.lo' failed
make[4]: *** [gallivm/lp_bld_misc.lo] Error 1
make[4]: Leaving directory '/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Build/Mesa/src/gallium/auxiliary'
Makefile:2255: recipe for target 'all-recursive' failed
make[3]: *** [all-recursive] Error 1
make[3]: Leaving directory '/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Build/Mesa/src/gallium/auxiliary'
Makefile:579: recipe for target 'all-recursive' failed
make[2]: *** [all-recursive] Error 1
make[2]: Leaving directory '/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Build/Mesa/src/gallium'
Makefile:667: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/home/mcatanzaro/src/WebKit/WebKitBuild/DependenciesGTK/Build/Mesa/src'
Makefile:607: recipe for target 'all-recursive' failed
make: *** [all-recursive] Error 1
*** Error during phase build of mesa: ########## Error running make -j 9 *** [31/37]
Comment 1 Michael Catanzaro 2015-11-21 15:56:58 PST
We build mesa with --enable-xlib-glx, but I don't know why. Anyway, that option now conflicts with --enable-egl. Not sure how to handle this....
Comment 2 Michael Catanzaro 2015-11-21 18:28:34 PST
(In reply to comment #1)
> We build mesa with --enable-xlib-glx, but I don't know why. Anyway, that
> option now conflicts with --enable-egl. Not sure how to handle this....

Well, it works
Comment 3 Michael Catanzaro 2015-11-21 18:31:43 PST
Created attachment 266036 [details]
Patch
Comment 4 Carlos Garcia Campos 2015-11-22 00:10:35 PST
The idea is that mesa uses the llvm from the distro, because we only build llvm for x86_64, but mesa is always built. So, even if we upgrade mesa version I would not add llvm as dependency and would build it before llvm.
Comment 5 Michael Catanzaro 2015-11-22 09:07:44 PST
(In reply to comment #4)
> The idea is that mesa uses the llvm from the distro, because we only build
> llvm for x86_64, but mesa is always built. So, even if we upgrade mesa
> version I would not add llvm as dependency and would build it before llvm.

We can't do that, because llvm does not have a stable API, so building mesa without llvm is a recipe for breakage (as we see here). We should either build llvm unconditionally and first, or else just use the distro-provided mesa that's guaranteed to be compatible with the distro-provided llvm. But I presume we are building mesa because it affects layout tests, which implies that we should keep building it, which implies that we should build llvm unconditionally.
Comment 6 Martin Robinson 2015-11-22 11:00:35 PST
(In reply to comment #5)
> (In reply to comment #4)
> > The idea is that mesa uses the llvm from the distro, because we only build
> > llvm for x86_64, but mesa is always built. So, even if we upgrade mesa
> > version I would not add llvm as dependency and would build it before llvm.
> 
> We can't do that, because llvm does not have a stable API, so building mesa
> without llvm is a recipe for breakage (as we see here). We should either
> build llvm unconditionally and first, or else just use the distro-provided
> mesa that's guaranteed to be compatible with the distro-provided llvm. But I
> presume we are building mesa because it affects layout tests, which implies
> that we should keep building it, which implies that we should build llvm
> unconditionally.

Yes, we are building Mesa because we want all layout tests on every system to run under the same version of llvmpipe.
Comment 7 Michael Catanzaro 2015-12-02 09:07:13 PST
Created attachment 266446 [details]
Patch
Comment 8 Michael Catanzaro 2015-12-02 09:08:08 PST
(Still need to run layout tests to see what's broken.)
Comment 9 Darin Adler 2015-12-02 16:57:40 PST
Comment on attachment 266446 [details]
Patch

rs=me
Comment 10 Michael Catanzaro 2015-12-03 03:32:44 PST
Thanks for the rs. I'm going to run layout tests on our test bot before committing. I'm also going to try building without --enable-xlib-glx and --disable-dri instead of using --disable-egl.
Comment 11 Michael Catanzaro 2015-12-03 03:33:18 PST
Created attachment 266526 [details]
Patch
Comment 12 Michael Catanzaro 2015-12-03 09:29:28 PST
Created attachment 266532 [details]
Patch
Comment 13 Carlos Alberto Lopez Perez 2015-12-03 10:07:36 PST
Comment on attachment 266532 [details]
Patch

Why we need libdrm at all when we are only interested in building a software rasterizer?
Comment 14 Michael Catanzaro 2015-12-04 02:00:06 PST
(In reply to comment #13)
> Comment on attachment 266532 [details]
> Patch
> 
> Why we need libdrm at all when we are only interested in building a software
> rasterizer?

So that we don't have to use --disable-egl, which conflicts with --enable-xlib-glx.

My understanding is that if we disable EGL, then layout tests will run with system EGL under Wayland, and we won't be able to guarantee consistent results in Wayland.
Comment 15 Michael Catanzaro 2015-12-04 02:24:39 PST
Although to be clear, Martin and I believe that currently we do not use GL at all under Wayland (since we have GLES disabled by default).
Comment 16 Michael Catanzaro 2015-12-04 04:11:47 PST
Created attachment 266620 [details]
Patch
Comment 17 Michael Catanzaro 2015-12-04 04:22:08 PST
(In reply to comment #14)
> My understanding is that if we disable EGL, then layout tests will run with
> system EGL under Wayland, and we won't be able to guarantee consistent
> results in Wayland.

We're going to try building mesa with --disable-egl instead.
Comment 18 Martin Robinson 2015-12-04 04:29:36 PST
Comment on attachment 266446 [details]
Patch

Michael says that he will wait to land this until he ensures that the new version of llvmpipe doesn't break tests.
Comment 19 Michael Catanzaro 2015-12-07 02:14:56 PST
Comment on attachment 266446 [details]
Patch

Having trouble running tests on the gardener, so we decided to land this and just watch the bots... I'm more than half expecting breakage.
Comment 20 Michael Catanzaro 2015-12-07 02:31:39 PST
*** Bug 151935 has been marked as a duplicate of this bug. ***
Comment 21 WebKit Commit Bot 2015-12-07 03:01:29 PST
Comment on attachment 266446 [details]
Patch

Clearing flags on attachment: 266446

Committed r193618: <http://trac.webkit.org/changeset/193618>
Comment 22 WebKit Commit Bot 2015-12-07 03:01:32 PST
All reviewed patches have been landed.  Closing bug.
Comment 23 Michael Catanzaro 2015-12-07 08:28:27 PST
This broke all layout tests.
Comment 24 Michael Catanzaro 2015-12-07 10:06:22 PST
Carlos Lopez found the bug; it is that LLVM got installed to lib instead of lib64. The fix will land as a side-effect of the patch in bug #145697.