[Mesa-dev] [PATCH v2 2/5] intel: Move tools/decoder.[ch] to common/gen_decoder.[ch].

[Kenneth Graunke]

On Tuesday, March 21, 2017 4:40:26 AM PDT Emil Velikov wrote:
> On 21 March 2017 at 11:14, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> > On 20 March 2017 at 22:04, Kenneth Graunke <kenneth at whitecape.org> wrote:
> >> This way they become part of libintel_common.la so I can use them in
> >> the i965 driver.
> >> ---
> >>  src/intel/Makefile.sources                          | 2 ++
> >>  src/intel/Makefile.tools.am                         | 2 --
> >>  src/intel/{tools/decoder.c => common/gen_decoder.c} | 2 +-
> >>  src/intel/{tools/decoder.h => common/gen_decoder.h} | 6 +++---
> >>  src/intel/tools/aubinator.c                         | 2 +-
> >>  5 files changed, 7 insertions(+), 7 deletions(-)
> >>  rename src/intel/{tools/decoder.c => common/gen_decoder.c} (99%)
> >>  rename src/intel/{tools/decoder.h => common/gen_decoder.h} (98%)
> >>
> >> I'd forgotten than I still had my gross symlinking hack in the first
> >> version of this series.  Here's a new spin which does this "properly" :)
> >>
> > You can do the symlink if you want to - I don't mind. This approach
> > will "bloat" every binary that links with libintel_common, since we
> > don't ask the linker to garbage collect.
> > Admittedly we only care about the ANV case, so I'll just follow-up and
> > add the GC toggle ;-)
> >
> Scratch that we do enabled it for ANV ;-)
> 
> You really nicely reworked [in 3/5] making the code "debug only", so
> perhaps can we guard the code in gen_decoder.c the same way ? ... But
> that may interact badly with aubinator :-\
> Another option would be to guard the code in ifndef ANDROID - like toggle.
> 
> Otherwise one will need to copy the _xml.h rules in Android, alongside
> a dummy libmesa_genxml-like library. The latter of which will need to
> be added as LOCAL_WHOLE_STATIC_LIBRARIES [for libmesa_intel_common] to
> pull/generate the headers. At the same time none of it is used since
> Android never defines DEBUG ... nor does it use the linker garbage
> collector (GC_SECTIONS)
> Tapani feel free to grep mesa for GC_SECTIONS and use in Android.

Ugh, good point - I forgot about Android (and SCons, but thankfully we
don't care about SCons here).  I don't have any clue how to test Android
builds, so I'm going to go ahead and land it as is (sorry!).

I think there's an easy fix: in Makefile.sources, rename COMMON_FILES
to MOST_COMMON_FILES (or some better name).  In Makefile.common.am, set
COMMON_FILES to MOST_COMMON_FILES and gen_decoder.c.  Then, the code
that needs the _xml.h rules won't get built.  It also won't be used
if Android doesn't define DEBUG.

> Either way, not my call - perhaps Tapani can weight in ?
> 
> IMHO the series looks great as-is and is
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
> 
> Emil

Thanks!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170321/8b454af8/attachment.sig>