[Mesa-dev] [PATCH 25/32] i965: Move gen6_blorp.c to a file that gets recompiled per-gen

[Jason Ekstrand]

On Mon, Aug 15, 2016 at 5:58 AM, Emil Velikov <emil.l.velikov at gmail.com>
wrote:

> Hi Jason,
>
> On 11 August 2016 at 22:15, Jason Ekstrand <jason at jlekstrand.net> wrote:
> > At the moment, it's only used for gen6 but that will change soon.  We use
> > the genX prefix for recompiled things in the Vulkan driver.  It isn't
> > great, but it seems to have worked ok.
> Speaking of genX and Vulkan - I was wondering where the split was
> since atm we do have both compile time and runtime genX "detection".
> With the GEN_VERSIONx10 macros the compiler/optimiser drops the
> unneeded codepaths from the final binary, although it [the binary] is
> larger if we compare it to runtime only checks.
>

95% of gen-specific stuff is compiled away but there are a few places where
recompiling is more trouble than it's worth.  I've thought about increasing
recompiling to the point where basically all VkDevice scopped calls are
recompiles and we just pick function table.  However, things seem to be
shaping up fairly nicely at this point and it's not clear that doing so
would actually be a benefit.


> Is the current approach chosen for runtime/speed purposes or it's
> mostly to work wit the movement of registers/fields across generations
> ?
>

Both.  The packing structs mechanism requires that we either recompile or
come up with some way of auto-generating the gen checks.  For the moment,
we've chosen recompiling and it's working fairly well.  It's not at all
clear that auto-generating the gen checks would actually result in less
code and it would certainly be harder.


> > ---
> >  src/mesa/drivers/dri/i965/Makefile.am       |  14 +-
> >  src/mesa/drivers/dri/i965/Makefile.sources  |   5 +-
> >  src/mesa/drivers/dri/i965/gen6_blorp.c      | 400
> ----------------------------
> >  src/mesa/drivers/dri/i965/genX_blorp_exec.c | 399
> +++++++++++++++++++++++++++
> >  4 files changed, 416 insertions(+), 402 deletions(-)
> >  delete mode 100644 src/mesa/drivers/dri/i965/gen6_blorp.c
> >  create mode 100644 src/mesa/drivers/dri/i965/genX_blorp_exec.c
> >
> Humble request: please run the following in your repo (or even with
> --global)
> $ git config diff.renames true
>
> Mauro, this change will break the Android build. Feel free to cook up
> a patch so that Jason can squash it here. This way we'll avoid the
> interment breakage.
>

Sure, I'd love a hunk to squash in.  If it looks reasonably like the ISL
patches, I should be able to do the copy+pasting for gen7, 7.5, 8, and 9
throughout the series.


>
> > diff --git a/src/mesa/drivers/dri/i965/Makefile.am
> b/src/mesa/drivers/dri/i965/Makefile.am
> > index 77ad1e8..35de8f9 100644
> > --- a/src/mesa/drivers/dri/i965/Makefile.am
> > +++ b/src/mesa/drivers/dri/i965/Makefile.am
> > @@ -48,11 +48,22 @@ brw_nir_trig_workarounds.c:
> brw_nir_trig_workarounds.py $(top_srcdir)/src/compil
> >         $(MKDIR_GEN)
> >         $(AM_V_GEN) PYTHONPATH=$(top_srcdir)/src/compiler/nir
> $(PYTHON2) $(PYTHON_FLAGS) $(srcdir)/brw_nir_trig_workarounds.py > $@ ||
> ($(RM) $@; false)
> >
> > -noinst_LTLIBRARIES = libi965_dri.la libi965_compiler.la
> > +I965_PERGEN_LIBS = \
> > +       libi965_gen6.la
> > +
> > +libi965_gen6_la_SOURCES = $(i965_gen6_FILES)
> > +libi965_gen6_la_CFLAGS = $(AM_CFLAGS) -DGEN_VERSIONx10=60
> > +
> > +noinst_LTLIBRARIES = \
> > +       libi965_dri.la \
> > +       libi965_compiler.la \
> > +       $(I965_PERGEN_LIBS)
> > +
> >  libi965_dri_la_SOURCES = $(i965_FILES)
> >  libi965_dri_la_LIBADD = \
> >         $(top_builddir)/src/intel/isl/libisl.la \
> >         libi965_compiler.la \
> > +       $(I965_PERGEN_LIBS)
> Missing trailing " \"
>

Oops... Not sure how that kept working...  Good catch!


>
> Thanks
> Emil
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160815/ce431fb0/attachment-0001.html>