Bug 217016
graphics/dri fails to build against devel/llvm40 but graphics/libGL and other Mesa ports don't really use LLVM.
In file included from draw/draw_llvm.c:45:
./gallivm/lp_bld_intr.h:69:20: error: unknown type name 'LLVMAttribute'; did you mean
'LLVMAttributeRef'?
LLVMAttribute attr);
^~~~~~~~~~~~~
LLVMAttributeRef
/usr/local/llvm40/include/llvm-c/Types.h:116:40: note: 'LLVMAttributeRef' declared here
typedef struct LLVMOpaqueAttributeRef *LLVMAttributeRef;
^
draw/draw_llvm.c:1577:10: error: implicit declaration of function 'LLVMAddAttribute' is invalid in
C99 [-Werror,-Wimplicit-function-declaration]
LLVMAddAttribute(LLVMGetParam(variant_func, i),
^
draw/draw_llvm.c:1578:27: error: use of undeclared identifier 'LLVMNoAliasAttribute'; did you mean
'LLVMAddAttribute'?
LLVMNoAliasAttribute);
^~~~~~~~~~~~~~~~~~~~
LLVMAddAttribute
draw/draw_llvm.c:1577:10: note: 'LLVMAddAttribute' declared here
LLVMAddAttribute(LLVMGetParam(variant_func, i),
^
draw/draw_llvm.c:2193:27: error: use of undeclared identifier 'LLVMNoAliasAttribute'; did you mean
'LLVMAddAttribute'?
LLVMNoAliasAttribute);
^~~~~~~~~~~~~~~~~~~~
LLVMAddAttribute
draw/draw_llvm.c:1577:10: note: 'LLVMAddAttribute' declared here
LLVMAddAttribute(LLVMGetParam(variant_func, i),
^
Mesa bug tracker shows similar issues apparently fixed in trunk, although I am not sure whether this applies to the specific compilation issue that Jan reports: https://bugs.freedesktop.org/show_bug.cgi?id=98627 https://lists.freedesktop.org/archives/mesa-dev/2016-November/134500.html The LLVM revision regarding removal of the deprecated Attribute API is at: http://llvm.org/viewvc/llvm-project?view=revision&revision=286062 Created attachment 180574 [details]
v0 (Mesa 17.0.0 + LLVM 4.0 by default but allow down to 3.6)
Thank you for the links. Backporting the specific fixes looked non-trivial, so I've tried to do a blind update. dri-17.0.0 builds fine with llvm36, llvm37, llvm38, llvm39 and llvm40. Runtime against llvm40 also appears fine as tested on drm-next (i915kms, Skylake, SNA) using a few games (OpenRA, Xonotic, PPSSPP), mpv (opengl-hq + VAAPI) and firefox (OpenGL vs. WebRender compositing).
Care to try? MESA_LLVM_VER can be overriden via make.conf(5) or Makefile.local.
I'm afraid my testing time is limited right now as I'm flying to an event in two hours, but I can confirm that libGL and libglapi built and installed with llvm40, as well as the packages my system uses which depend on them, libglut and freeglut. llvm39 was not required unless I set MESA_LLVM_VER in make.conf Thanks for putting this patch together! One thing I forgot to mention: I did get this showing up twice in a row during the configure step, just before "configure: creating ./config.status": Package 'libva', required by 'virtual:world', not found Package libva was not found in the pkg-config search path. Perhaps you should add the directory containing `libva.pc' to the PKG_CONFIG_PATH environment variable I honestly don't know if that's normal; I figured I should mention it. This is a server, so libva may not even be useful for it. Created attachment 180617 [details]
v0.1 (Mesa 17.0.1 + LLVM 4.0 by default but allow down to 3.6)
Created attachment 180618 [details]
v0.2 (Mesa 17.0.1 + LLVM 4.0 by default but allow down to 3.6)
Allowing more flexibility of the LLVM version to allow those who are ahead to use the one they currently have installed, i.e. llvm40 instead of llvm39 at the moment, is a good idea. Allowing use of older versions of LLVM is not such a good idea; the result will be degraded functionality, if it works at all. Support of newer OpenGL versions and newer hardware, i.e. amdgpu, requires fairly current LLVM. Just because Mesa could be built with 3.6 that does not mean it would be fully functional. Also, the LLVM port needs to provide a library for Mesa to use, but as it is not built by default it is not provided by all the llvm ports. llvm36 might provide the library, I haven't checked, it's ancient. llvm37 does provide the library the old way, meaning the shared LLVM lib is built and all the LLVM tools link to it. LLVM 3.8 introduced the option to build the shared library without linking anything to it, which is the recommended way as the static build of LLVM is more performant than a dynamic linked build. I experimented with both llvm38 and llvm39 ports some moths ago. llvm39 was patched to provide the necessary shared library which is now used by almost all OpenGL and OpenCL ports. llvm38 should be able to provide the shared library with the same changes, but the build failed on all my attempts, so I punted on it given that llvm39 was already available and working. The rather specific BUILD/RUN_DEPENDS on 3.9.0_4 is to ensure that the version of the llvm39 port in use provides the required shared library, prior port revisions did not. Therefore, we would need a version whitelist to safely allow use of older llvm ports. That would be be a good deal of unnecessary complication for little if any benefit, and that only addresses Mesa. For OpenCL there are a number of ports other than Mesa which have their own, narrower version requirements for LLVM, not all of which can be satisfied with a single version, but as seen after the last update there can be a problem with multiple LLVM versions installed (numerous people reported failures in Mesa when both llvm37 and llvm39 were present, cured by removing llvm37, probably related to the shared lib from llvm37 which had extra unnecessary consumers that may have caused it to get preloaded). Additionally, this all would go counter to the effort to clear old llvm ports of dependents. That said, a patch to require llvm39 but use llvm40 if it is already installed would be nice, but that is not what the proposed patch does. Instead, it merely allows MESA_LLVM_VER to be more easily overridden while also dropping the min version and bumping the default version, dropping part of post-patch, rolling the post-patch changes or a subset thereof into patch-configure and patch-configure.ac (which need not be patched), and there is some other change to configure patches I have no looked at close enough to see whether the make sense for the 13 -> 17 bump given the rest is incorrect. Mesa 17.0 will either simply require llvm40 or it will require llvm39 and accept llvm40. I have been testing 17.0 built with llvm39 so far while watching the LLVM 4.0 release get pushed back. Given 4.0 is not yet released and there are numerous other ports to consider, many of which just moved to llvm39 if not still stuck further behind, it is probably imprudent to rush to require llvm40 for Mesa 17.0. Mesa 17.1, which is at least a month away, will require llvm40. We can be sure llvm40 will be in good shape by then, other parts of the graphics stack that may not yet have caught up should be by then, and to not use LLVM 4.0 would forgo some of the benefit of upgrading Mesa to that point. (In reply to Matthew Rezny from comment #8) > That said, a patch to require llvm39 but use llvm40 if it is already installed > would be nice, but that is not what the proposed patch does. Detecting dependencies at runtime is undesired. https://www.freebsd.org/doc/en/books/porters-handbook/makefile-depend.html#makefile-automatic-dependencies > Instead, it merely allows MESA_LLVM_VER to be more easily overridden > while also dropping the min version and bumping the default version I wanted the knob hidden, so it can allow footshooting. Not really a feature supported by the maintainer. > dropping part of post-patch, [[:graph:]]* wasn't dropped but plain incorrect. According to re_format(7) negating character class is perfectly fine but BSD regex doesn't support shorthand variants. This may change with the import of libtre in future. I do use CPUTYPE?=native in make.conf myself, so this was tested. > rolling the post-patch changes or a subset thereof into patch-configure and > patch-configure.ac (which need not be patched), 17.0.1 requires more extensive changes. I've tried to use the syntax compatible with both GNU sed and BSD sed. files/configure.ac was out of date, so it should either be removed or submitted upstream. Leaving it as is is confusing. https://cgit.freedesktop.org/mesa/mesa/commit/?id=9ab1704f42b > and there is some other change to configure patches I have no looked > at close enough to see whether the make sense for the 13 -> 17 bump > given the rest is incorrect. What else? For one, the following change was made to be consistent with the rest of the file. -+linux*|freebsd*) ++linux* | freebsd*) If you didn't notice my changes in files/configure.ac are ready to be submited upstream (support both GNU and BSD sed). It's just getting involved with upstream review is a bit too time-consuming for me. I'm already exhausted with upstreaming stuff in Firefox. (In reply to Jan Beich (mail not working) from comment #9) > Detecting dependencies at runtime is undesired. I was thinking of this akin to selecting the correct compiler and trying to accommodate the original request, but if we are looking at it in terms of dependency handling then it should just be a one line patch to change MESA_LLVM_VER=39 to MESA_LLVM_VER?=39. >I wanted the knob hidden, so it can allow footshooting. Not really a feature >supported by the maintainer. If someone really wants to shoot their foot off, they can relax or drop the version checks entirely, but the maintainers need not invite accidental foot shooting with an a relaxed version requirement. >[[:graph:]]* wasn't dropped but plain incorrect. According to re_format(7) >negating character class is perfectly fine but BSD regex doesn't support >shorthand variants. This may change with the import of libtre in future. >I do use CPUTYPE?=native in make.conf myself, so this was tested. That was what was suggested to me and it worked so I did not scrutinize it. I saw it disappear with no explanation given so it appeared to have been dropped. Thank you for stating the reason. I guess it should have been [:space:] instead of [:graph:] in the replacement >17.0.1 requires more extensive changes. I've tried to use the syntax compatible >with both GNU sed and BSD sed. files/configure.ac was out of date, so it should >either be removed or submitted upstream. Leaving it as is is confusing. Now that you explain that, it makes more sense. I was evaluating the proposed patch from the point of view of the original request, what is the minimum change to accomplish that. The patch had a lot more going on, and at glance and without any statement as to why those changes were made, it looked like post-patch bleed into makepatch again. I was not wanting to patch configure.ac to avoid an otherwise unnecessary autoreconf, but if we can patch the .ac and upstream that then it certainly makes sense to do so. >What else? For one, the following change was made to be consistent with the >rest of the file. No problem with the style change there, and there wasn't necessarily anything wrong with the parts I didn't mention, just that I did not compare them to my Mesa 17 WIP after tripping over the sed changes. Perhaps incorrect wasn't the right label, it was more of unexplained changes that looked incorrect on the surface. (In reply to Jan Beich (mail not working) from comment #10) >If you didn't notice my changes in files/configure.ac are ready to be submited >upstream (support both GNU and BSD sed). It's just getting involved with >upstream review is a bit too time-consuming for me. I'm already exhausted with >upstreaming stuff in Firefox. No, I didn't notice, but now that you state the purpose, I thank you for doing so. I plan to upstream as much as possible once I have us on 17.0.x, Emil recently contacted us requesting we send up what we have. I just have to ask, if the patched configure script works with BSD and GNU sed, do we still need the bit for DragonFly in post-patch? I intend to restructure the Mesa ports during the update to 17. It does not make sense to have libGL as the master, and it might not make sense to still have a separate libGL. In order to enable Wayland support for libEGL without ending up with Mesa always depending on part of Wayland requires adding an option, but same option would need to be present in gbm and setting the option differently there would resulting in a broken system. The simplest solution to guarantee consistency is to make gbm part of libEGL. I only found one port that requires gbm but not libEGL, so the combination should be inconsequential. Of course, doing that, I can't help but think how much simpler it would be to have libGL, libEGL (with gbm), libglesv2, and libglapi in a single mesa-libs port which any (E)GL(ES) user would depend on. The only separate parts would be mesa-drivers aka dri, clover, and osmesa. Most consumers depend on libGL, which is the majority of the size and already would pull in libglapi and gbm with it. Adding in libEGL and libglesv2 is an increase from ~2MB to 3MB installed, so harmless and hardly worth separating the libs into numerous ports/packages that require rebuilding some parts several times and complicate options. Maybe I should make a meta-port simply called mesa to serve as the master and optionally depend on the rest. Input on the idea is welcome. (In reply to Matthew Rezny from comment #11) > I was not wanting to patch configure.ac to avoid an otherwise unnecessary autoreconf files/configure.ac has to be renamed to files/patch-configure.ac to produce any effect. My guess files/configure.ac was added in ports r371035 as a reference for future changes to files/patch-configure. As ports r433862 removed libdevq logic in favor of ports r431708 files/patch-configure is now trivial to modify again. > if the patched configure script works with BSD and GNU sed, do we still need > the bit for DragonFly in post-patch? DragonFly (and NetBSD) lack base r268066 (since FreeBSD 10.1-RELEASE), see my inline comment. OpenBSD appears to have it, and their re_format(7) manpage is superior. http://man7.org/linux/man-pages/man7/regex.7.html http://man.openbsd.org/re_format > I can't help but think how much simpler it would be to have libGL, > libEGL (with gbm), libglesv2, and libglapi in a single mesa-libs > port which any (E)GL(ES) user would depend on. The only separate > parts would be mesa-drivers aka dri, clover, and osmesa. +1 on "mesa-" prefix to avoid confusion where "libGL and friends" notion starts and ends when communicating updates to uninitiated users. +1 on simplifying build but we can restore some granularity via subpackages[*] later. [*] https://lists.freebsd.org/pipermail/freebsd-ports/2016-December/106315.html (In reply to Jan Beich (mail not working) from comment #10) https://cgit.freedesktop.org/mesa/mesa/commit/?id=fe56c745b8cb Created attachment 181003 [details] v0.3 (Mesa 17.0.1 + LLVM 4.0 by default but allow down to 3.6) http://lists.freedesktop.org/archives/mesa-dev/2017-March/148702.html http://lists.llvm.org/pipermail/llvm-announce/2017-March/000073.html "poudriere bulk -t" (8 ports) is green on 10.3 i386/amd64, 11.0 i386/amd64, 12.0 amd64. Runtime tested in 11.0 amd64 jail with i965 (Gen9) and llvmpipe. A commit references this bug: Author: rezny Date: Wed Mar 29 16:57:54 UTC 2017 New revision: 437215 URL: https://svnweb.freebsd.org/changeset/ports/437215 Log: Update to 13.0.6 - Allow use of newer LLVM, i.e. llvm40, via MESA_LLVM_VER in make.conf [1] - Disable use of LLVM on platforms where it's known not to be available [2] PR: [1] 217016, [2] 216944 Reported by: [1] greenreaper@hotmail.com, [2] linimon Approved by: swills (mentor) Differential Revision: https://reviews.freebsd.org/D10183 Changes: head/graphics/gbm/Makefile head/graphics/libEGL/Makefile head/graphics/libGL/Makefile.common head/graphics/libGL/Makefile.targets head/graphics/libGL/distinfo head/graphics/libglapi/Makefile Created attachment 181302 [details] v0.3 (Mesa 17.0.2 + LLVM 4.0 by default but allow down to 3.6) Rebased. ports r437215 prematurely advertised MESA_LLVM_VER=40 despite comment 1 which bug 218196 also tries to fix. Created attachment 181306 [details]
v0.3 (Mesa 17.0.2 + LLVM 4.0 by default but allow down to 3.6)
Rebase oops, forgot to remove -e 's|\\S\*//|[:space:]* //|' in favor of upstream version for FreeBSD as well.
*** Bug 218251 has been marked as a duplicate of this bug. *** Created attachment 181391 [details] v0.4 (Mesa 17.0.3 + LLVM 4.0 by default but allow down to 3.6) https://lists.freedesktop.org/archives/mesa-announce/2017-April/000315.html My QA is same as in comment 3 + llvm-devel (5.0.d20170329_1), intel(4) + DRI3, modesetting(4), llvmpipe (Gallium). YMMV Comment on attachment 181391 [details]
v0.4 (Mesa 17.0.3 + LLVM 4.0 by default but allow down to 3.6)
poudriere bulk -t (8 ports) is green for 10.3 i386 + amd64, 11.0 i386 + amd64, 12.0 amd64; unrelated to QA.
Hello, Just wondering, since 11-STABLE has now LLVM 4.0.0, isn't that good enough? Currently building graphics/dri by default pulls both devel/llvm39 and devel/llvm40 thanks to devel/libclc. devel/llvm39 stays since it's a direct dependency for libEGL and dri. (In reply to jakub_lach from comment #21) LLVM is base is incomplete. Mesa needs a shared library that is only built by the LLVM port. The update of libclc was a bit premature, it would have been better to save that until Mesa 17 was ready to be committed so that they would both depend on llvm40, but the situation will improve and soon enough you'll be able to uninstall llvm39. Thanks for clarification. I don't mind building extra compilers if that's the only way forward, though ideally I would like to have only one compiler around, like in the old times, but unless base gets modular or base one will have needed libs, that idea is shelved for now. Obsoleted by ports r438198. I'll move LLVM 5.0 bits (to prepare for 17.1) into a separate bug later. |
Created attachment 179885 [details] llvm39 being installed by upgrading 'libglapi-11.2.2' to 'libglapi-13.0.4' I recently upgraded to FreeBSD 10.3. I installed llvm40 from ports and recompiled all other ports. Now, I am trying to keep the system up to date using llvm40. /usr/ports/graphics/libGL/Makefile.common contains the line: MESA_LLVM_VER=39 This forces a portupgrade of libglapi to install llvm39 for the recent update. libglapi and libGL compile and install with llvm40 if I change MESA_LLVM_VER. I think libGL should work with either port, and llvm41 once it exists, etc.