On 23 January 2017 at 19:39, Tobias Droste <[email protected]> wrote: > Am Montag, 23. Januar 2017, 11:53:18 CET schrieb Jose Fonseca: >> On 20/01/17 02:48, Emil Velikov wrote: >> > On 19 January 2017 at 19:26, Tobias Droste <[email protected]> wrote: >> >> Am Mittwoch, 18. Januar 2017, 18:45:04 CET schrieb Emil Velikov: >> >>> On 18 January 2017 at 18:12, Jose Fonseca <[email protected]> wrote: >> >>>>>> In order to untangle things we want to have a distinction between the >> >>>>>> gallium (gallivm afaict) and other users - RADV presently. >> >>>>>> So how about we update the RADV instances and ensure that the we set >> >>>>>> the HAVE_{RADV,}_LLVM lot appropriately. Latter will be picky but >> >>>>>> overall things should work w/o annoyances that HAVE_GALLIUM_LLVM >> >>>>>> brings ? >> >>>> >> >>>> I honestly don't even understand why we'd want to build parts of the >> >>>> tree >> >>>> with LLVM while hiding LLVM from other components. We can't we just >> >>>> build >> >>>> everything with LLVM and avoid this combinatorial explosion of wierd >> >>>> options that are nothing more than yet another way the build can >> >>>> break!!? >> >>> >> >>> Sadly the combinatoric explosion has been there for a while. Based on >> >>> how well my previous attempts to resolve similar issues (see the >> >>> "platforms" topic) I doubt we'll even get to fix that. >> >>> >> >>>> But if a separate option is truly necessary, have the newcomer pick a >> >>>> different name, or something. >> >>> >> >>> That's pretty much what I suggested above. Tobias can you please give it >> >>> a >> >>> try ? >> >> >> >> I would rather "fix" the other build systems. (As in just define >> >> HAVE_GALLIUM_LLVM if HAVE_LLVM is defined). >> >> >> >> I think there is still a misunderstanding on Joses side on what this >> >> really >> >> means. No file in gallivm or llvmpipe will be touched. It's really just >> >> auxilliary/draw and there it's exactly 8 lines that will change. >> >> >> >> That's it. >> >> >> >> I really fail to see how this will break everything that is being worked >> >> on >> >> and cause merge conflicts everywhere. >> >> >> >> If you still want the other way, I can do that to, but this will of >> >> course >> >> need the same fix in the other build system or we have the same situation >> >> we have now, but with other drivers. >> > >> > Afaict one point is that the use of HAVE_GALLIUM_LLVM vs HAVE_LLVM is >> > too subtle. Let's not forget that barring the WIP(?) branches, VMWare >> > has closed source components. Guess how much fun it will be as >> > suddenly things fail to build/work properly as they re-sync the code >> > base. No idea how likely the latter is, but considering Jose (and a >> > few other VMWare guys) wrote sizeable hunk of that code (and Mesa as a >> > whole) I'd go with his instinct. >> > >> > Emil >> >> The HAVE_LLVM->HAVE_GALLIUM_LLVM rename is indeed not as invasive as I >> thought. >> >> But I still don't understand why HAVE_LLVM->HAVE_GALLIUM_LLVM is >> necessary in draw and not on gallivm/llvmpipe. >> >> People want to build draw with LLVM support, but without >> gallivm/llvmpipe? That's impossible. >> >> Or is this because the draw files are the only .c files that are >> compiled even when HAVE_LLVM is undefined, so these are the only ones >> that get to receive the renaming treatment? That's crazy confusing. >> There's no away I can accept that. >> > > The draw files are used by softpipe (and maybe other gallium drivers, haven't > checked that) and there HAVE_LLVM should not be defined. If it's not, > everything is fine. But with new non gallium drivers using LLVM and causing > HAVE_LLVM to bedefined, softpipe is broken in some cases. See below. > >> >> Let me make this crystal clear to avoid making this discussion even more >> protracted: I will not accept any HAVE_LLVM change in >> draw/gallivm/llvmpipe .C/.H source code. Period. >> > > I'm _not_ changing gallivm and llvmpipe. Draw is not only used by llvmpipe and > I still think I have very good arguments for the change. See again below. > > I understand, I'm the unknown new guy and you did a lot of work in this code. > But I'm not getting paid for this and I don't have to do this. I want to help, > but I also want to understand why I can't do something. With reasons other > than "I say so, and I don't want to hear any reasons against it". I hope you > understand that. > >> >> HAVE_LLVM used to mean, "whole Mesa being built with LLVM". Now people >> want to build something (no idea what yet to be honest) with LLVM, but >> not build draw/gallivm/llvmpipe. >> >> If you want to build some other component with LLVM but not >> draw/gallivm/llvmpipe, then add a new HAVE_LLVM_FOR_FOOBAR define and >> use it where you need it. > > The real problem is softpipe. Softpipe uses draw but (obviously) can't use > LLVM. > > Right now one could build radv (uses LLVM) and pass --disable-gallium-llvm to > the build system to get softpipe built. > > But due to radv "HAVE_LLVM" (which is used as a version check everywhere > else!) is defined and the draw code gets build using gallivm (since HAVE_LLVM > is defined). So the resulting softpipe will actually use LLVM even though you > deselected LLVM for gallium. > > That's how it is right now! > > The draw code is the only code that has optional LLVM support and also the > only code that uses HAVE_LLVM without any version check just to see if it > should use LLVM or not. That's why this is the obvious and easy to change > component. > Trying to reword things: - optional build against LLVM has been around since day 1, afaict. - we have code (galliuvm, llvmpipe, etc.) which is built only when llvm is present and other (draw) which has conditional ifdef HAVE_LLVM hunks - latter of ^^ is required since we want to have softpipe only (i.e. w/o llvmpipe) even though it uses draw - [side note] other drivers (such as r300 and nouveau, nv30 in particular) also use draw - the macro name was changed only in draw, since everything else is build _only_ if we have LLVM. Outside of draw, HAVE_LLVM _is_ a misnomer since we use it it as a version check, rather than presence. - some parts (radv, others?) do not use draw therefore the configure toggle and thus the definition of HAVE_LLVM are off. HAVE_LLVM has the meaning of USE_LLVM_FOR_DRAW within draw.
> I'm open for a rename. USE_LLVM_FOR_DRAW? > I would still argue that this is the right place to do the change! > Fwiw I agree that renaming it in gallium/draw would be better. There's a couple of things though: - it should be pretty clear (or otherwise one can get the whole thing within 2 mins or so) what this new define is a why/where would one need it. Perhaps adding a few comments throughout and/or a small README ? - one has to get the devs that work on that code (git log) behind this. With the above sorted this shouldn't be too hard, imho. > But having said all that: > If we're all okay to disallow a mixed LLVM/no-LLVM build, I'm fine with that > too. In that case there's no renaming to be done. > Right now that sould only break configurations where radv is somehow involved > as all the other LLVM users are gallium drivers as far as I can tell. > > Emil? :-) > You're suggesting that we force require --enable-gallium-llvm when building RADV ? Not too big of a fan I'm afraid. If possible I'd leave that as the final option. Thanks Emil _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
