On 07/04/15 17:16, Matt Turner wrote:
On Tue, Apr 7, 2015 at 5:14 AM, Jose Fonseca <jfons...@vmware.com> wrote:
Sorry for the delay. I've been away during the Easter.

On 02/04/15 19:02, Matt Turner wrote:

On Thu, Apr 2, 2015 at 7:32 AM, Jose Fonseca <jfons...@vmware.com> wrote:

These were being defined in SCons, but it's not practical -- we actually
need to include Gallium headers from external source trees, with
completely disjoint build infrastructure, and it's unsustainable to
replicate the HAVE_xxx checks or even hard-coded defines across
everywhere.


To confirm, you're building external sources with gcc? I don't think
these macros are useful for MSVC.


Correct.



No actual change in behavior for autoconf.
---
   configure.ac         |  2 +-
   include/c99_compat.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
   scons/gallium.py     | 27 ---------------------------
   src/util/macros.h    |  2 ++
   4 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/configure.ac b/configure.ac
index 520cc22..1485bba 100644
--- a/configure.ac
+++ b/configure.ac
@@ -230,7 +230,7 @@ _SAVE_LDFLAGS="$LDFLAGS"
   _SAVE_CPPFLAGS="$CPPFLAGS"

   dnl Compiler macros
-DEFINES=""
+DEFINES="-DHAVE_AUTOCONF"
   AC_SUBST([DEFINES])
   case "$host_os" in
   linux*|*-gnu*|gnu*)
diff --git a/include/c99_compat.h b/include/c99_compat.h
index 4fc91bc..62ccd46 100644
--- a/include/c99_compat.h
+++ b/include/c99_compat.h


c99_compat.h doesn't seem like the right location. I know it seems
like a nice place to add this since it's included everywhere, but I
worry that in a few years we're going to be cleaning it up like we've
been doing with compiler.h and friends.

I might make a separate header to define these? Not sure.


I can move the defines out of c99_compat.h , e.g.,
mesa/include/fallbackconfig.h.

But I'd prefer to include fallbackconfig.h out of c99_compat.h , as
c99_compat.h is pretty much guaranteed to be included all the time.


Since
probably all cases of #ifdef HAVE___* have a fallback, that runs the
risk of never noticing that you weren't including the right header.

Precisely, this is all the more reason why it must be included from a header
that's included all the time.  If it depends on people to add the include on
a case-by-case it is bound to fail, as nobody else but us cares, and it will
easily go unnoticed.


@@ -141,4 +141,49 @@ test_c99_compat_h(const void * restrict a,
   #endif


+
+/* Fallback definitions, for when these headers are used by build
systems which
+ * don't auto-detect these things.*/
+#ifndef HAVE_AUTOCONF


I'd rather flip this condition around and not modify configure.ac. But
maybe you can't do that because you're not actually building
everything with scons?


No biggie either way.

I don't know. This seems nuts. I really don't like adding stuff to the
autotools build system like this.


Sure.


I really don't know how to deal with this. What I'm hearing is that
even the custom scons build system you guys use isn't sufficient for
your own needs. You're not building the external source trees with the
same build system...?


I think you might be getting the wrong idea.

We don't build the .C files from external source trees.  But we do need to
include .h files, so we can interface with components in Mesa tree.

That is, I only need the .h files to make sense on their own (with Mesa
components, namely mesa/src/gallium/include, and gallium auxiliary
libraries).  But we have so many inlines functions, so many #ifdef HAVE_foo,
that unless all the defines match precisely, the whole hell breaks loose.


Gallium has from the start been integrated (ie. embedded) on a myriad of
places.  It was always meant as a framework to write any sort of 3d driver,
not just OpenGL drivers.  Things were much worse when Gallium was used on
Windows XP kernel land or Windows CE.  I'm glad that I or anybody else has
to deal with the quirkiness of keeping code portable across these platforms.
Things are still much more uniform nowadays.


I mean, in all the build system work I've done I've tried to make sure
scons continues working -- doing things like adding these HAVE_*
definitions to it and such. It's kind of frustrating, and it's even
more frustrating when even that isn't sufficient.



All I'm doing here is basically move your defines out of scons's python
files into C headers.  Conceptually it's doing pretty much the same thing as
before, but being in a header that means that it's there for all build
systems to take.


Rembember that Mesa itself is not just autoconf and Scons, there's also
Android build system.

I don't like it any more you do, but this is the world we live in: the fact
is that many platforms constraint how software must be built to a point
which is impracticable/impossible to build.  Even if a build system that
meets everybody needs existed, we'd still face the legacy of existing
software using other build systems.



To be honest, IMHO, Mesa source tree and build systems are a failure if they
can't even sustain external interfaces.


For many drivers, the external interface headers are Khronos OpenGL / GLES
headers.  But for gallium drivers, the interface is mesa/src/gallium/include
(plus some .h from helper modules in src/gallium/auxiliary as it is
impractical to interface with gallium drivers without them.)


What would you say in a parallel reality, Khronos demanded that in order to
build OpenGL drivers for Linux one would need to use the Khronos own build
system?  Because that's basically what's at stake here: if I want to
interface with gallium and llvmpipe driver should I be forced to build my
code with Mesa build system?


So I only see three ways of dealing with this:

a) have fallback HAVE_* foo from the headers (so that all inline functions
compile the same way) as I propose in this patch

b) move all inline functions to separate headers (so that external code can
opt-out from including them), and provide alternative non-inline
implementations (so that external code can still call them)

c) stop using inline functions altogether



One way or the other, we'll need the headers to make sense on their own,
without having to duplicate the whole Mesa build-systems.  But b) and c) can
have performance impact. (Particularly because we really want to inline
atomic reference counting.)



Jose

Thanks Jose.

Thanks for explaining the reality of the situation. It makes sense to me.
>
I'm fine with these being added to c99_compat.h or another header
included by it. I think my only real change request is to invert the
macro to not have to modify configure.ac.

Sure.  I'll update that patch then.

I'll submit the other 1-3 patches of these series as before that, as they are minor cleanups and not controversial.

In your follow-up email you mention autoconf generating config.h. I've
thought about that as well, but I didn't consider that it might allow
us to solve this problem (I'm not entirely sure it would, since it
seems like you'd have to run configure to generate config.h and *then*
run scons and your out-of-tree build system).

What I had in mind was that SCons config.h would be hand-written (ie, basically contain everything that I added to c99_compat.h in the first version.)

So autoconf would generate its own config somewhere, and add `-Ipath/to/where/generated/config_h/goes`, while scons would have `-Ipath/to/where/handwritten/fake/config_h/lives`


In any case, that might be a nice solution. I'll think about that some more.

Thanks,
Matt


Thanks.

Jose
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to