On 24/01/17 13:11, Emil Velikov wrote:
On 23 January 2017 at 19:39, Tobias Droste <tdro...@gmx.de> 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 <tdro...@gmx.de> wrote:
Am Mittwoch, 18. Januar 2017, 18:45:04 CET schrieb Emil Velikov:
On 18 January 2017 at 18:12, Jose Fonseca <jfons...@vmware.com> 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
I can't condone this plan. From where I'm standing you are:
1) trying to build part of mesa w/ LLVM and others without LLVM, which
is IMO just bad and shortsighted.
Sooner or later, more and more Mesa components will depend on LLVM.
Do people seriously want to cherry-pick the Mesa components use LLVM, by
adding HAVE_LLVM_FOR_FOO, HAVE_LLVM_FOR_BOO, ...!?
It's insane; it's not sustainable; and it promotes the wrong behavior
by developers (creating barriers for code reuse by putting things in silos.)
2) you're keeping things nice and neat for yourselves by shoving the
ugly bits (like this weird USE_LLVM_FOR_DRAW/HAVE_GALLIUM_LLVM) into
draw/gallivm/llvmpipe, which I simply cannot and will never accept.
I don't feel I have legitimacy to stop 1) regardless how awful I think
it is, but I'm not budging on 2).
So do whatever you want with RADV, but leave the HAVE_LLVM in
draw/gallivm/llvmpipe alone. If you truly want RADV to consume LLVM
independently of the rest of Mesa then you must achieve it within the
confines of RADV. You can't put the burden of dealing with that onto
the other components.
I think I engaged into this discussion for long enough. I honestly
tried to listen for a good reason to do what's being proposed, but none
was presented so far, and I feel we're all just repeating ourselves.
Jose
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
