[Mesa-dev] [PATCH 00/20] Auto-generate pack/unpack functions

Jason Ekstrand jason at jlekstrand.net
Tue Nov 18 09:10:52 PST 2014


Jose,
I haven't had time to fully review Iago and Samuel's code, so I can't 100%
comment on it right now.  However, let me make a few comments on the
"overarching plan" as it were.

On Tue, Nov 18, 2014 at 2:36 AM, Jose Fonseca <jfonseca at vmware.com> wrote:

> > The idea is that we have a lot of format conversion code scattered
> through
> > different files in the repository, a lot of that is redundant /
> duplicated,
> > so this intends to address that issue.
>
> First, I think this is a great goal.  And while I haven't reviewed them in
> detail, just from skimming through them, these patch series seem to be a
> great cleanup and a lot of work went into it.  I certainly don't object to
> any of this.
>
>
>
> But I have to say I find a bit unfortunate that so much effort is being
> put on implementing something specific to Mesa formats instead of taking
> this opportunity to devise a solution that would work both for gallium and
> Mesa formats.
>

That is the end goal.  Unfortunately, getting there requires a lot of
work.  Probably more work on the mesa side than on the gallium side.  A big
part of the problem was that there was a lot of code for format conversion
and it was scattered all over mesa/main directory.  A lot of stuff needs to
be cleaned up an unified inside mesa/main before things can be unified with
gallium.  Much of that work is now done.

One of the things that I would like to see happen after this stuff lands is
to convert the mesa pack/unpack functions to take a width, height, and
stride.  Then they would have exactly the same function signature as the
gallium conversion functions and merging will be much easier.  Then we can
work on moving the format handling code into a helper library which, for
the moment, I'll call libmesaformat.  Then both gallium and mesa classic
can pull from libmesaformat and we can kill all of the redundant code.
Whose autogenerator framework we end up keeping is kind of immaterial,
they're not that hard to write.

One of the decisions that has to be made there (probably a topic for
another thread) is how we would want to structure the format metadata.
Mesa and gallium both have completely different ways of structuring it and
we need to unify that if we're going to unify the conversion code.
Personally, I think gallium's is cleaner and more expressive, but it lacks
the GL information that core mesa needs.  But, like I said, that's a topic
for another thread.


> Furthermore I see there is some interest speeding mesa using SSE2
> intrinsics, and of course format conversion is precisely one of the code
> paths that can great benefit from SIMD, but I have no doubt: the most
> efficient and sane way of leveraging SIMD with all these formats conversion
> is to JIT compile format conversion code tailored to the CPU in runtime.
> There are just too many CPU variations to statically generate C code for
> every one of them.  And lot of the code to do this already exists in
> src/gallium/auxiliary/gallivm.  We should consider using it from src/mesa/*.
>

Yes, there were some patches.  However, given my experiments in the past,
I'm skeptical as to how much of a real benefit it would be to
SSE-accelerate all the format conversion.  When I did my first major rework
a couple of months ago, I experimented with using the SSSE3 shuffle
operation for doing swizzling of the C implementation.  The net result of
that experiment is that using SSSE3 had a very marginal benefit over a good
C implementation such as the one we now have.  The real problem we had was
that the current format conversion stuff was doing the pessimal thing in a
lot of cases;  a lot of that stupid is now gone.  So, if someone wants to
work on that, I'm curious to see their results, but I'm not holding out for
it.

As far as doing a JIT compile, I've thought of it.  Daniel Stone recently
mentioned the idea of using ORC for doing things like this and it might be
a good fit.  However, be fore we can do that, we need a framework for these
things (which we now have, thanks to this series).  How is that done in
gallivm?  Is it done using LLVM?  If so, it might be a non-starter.  I
don't want to rekindle any debates, but you're going to have trouble
convincing some people that format conversion is a good enough reason for a
hard dependency on LLVM.

Another idea that has been put forward would be to, whenever possible, push
the unconverted data to the GPU as a linear texture and use the GPU to do
the format conversion.  This would quite possibly be better than either a
straight CPU path or an optimized JIT'ed path.

This gallium helper code was written outside mesa to avoid the GL
> dependency (there was one point some of this stuff had to run in XP kernel
> mode! thankfully not anymore), and because when gallium was written the
> idea was that all Mesa drivers would eventually migrate into it.  Alas,
> that didn't happen, and might never happen.  That's OK.  But in that case,
> I do believe we should find a way of sharing more of the stuff in
> src/gallium/auxiliary with all Mesa drivers, non-gallium classic drivers
> included.
>
>
> In particular, we really should have a format conversion module that is
> capable of handling a superset of all Mesa and Gallium formats somwhere in
> src/util/  .  One might even leverage the auto-generated pack/unpack
> functions in src/gallium/auxiliary/u_format* though I wouldn't care if not,
> as long as the functionality is the same.
>

Absolutely!  That's 100% the end goal here.  One of the additions of this
series is a new pseudo-enum type called MESA_ARRAY_FORMAT.  Between that
and the MESA_FORMAT enum you can, with a single 32-bit integer, represent
all of the mesa formats, gallium formats, and the entire combinatorial
explosion of GL formats.  The "master conversion function" mentioned
several times in this series is capable of converting between any of these
formats.  Also, this is a mesa enum, not a GL enum.  The only knowledge the
conversion functions have of GL is a single function that converts a GL
format/type pair into a MESA_FORMAT or MESA_ARRAY_FORMAT.  From there on,
it's entirely GL agnostic.

I hope this clears up some of your reservations.
--Jason

In short, we can't change the past, but I wish we could have more synergy
> in the future.
>
>
> Jose
>
>
>
>
> On 18/11/14 08:43, Iago Toral Quiroga wrote:
>
>> This is the fist of two series of patches to address:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.
>> freedesktop.org_show-5Fbug.cgi-3Fid-3D84566&d=AAIGaQ&c=
>> Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=
>> zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=
>> 9H2UxyGUngIpuFNlQEzL9VtIOmYVKVs2o4nyL_We6o0&s=xYhgcKEBxp3-d_
>> ddjpFiNGRkqLo2BMUedLQ67ck2ce8&e=
>>
>>
>> The idea is that we have a lot of format conversion code scattered through
>> different files in the repository, a lot of that is redundant /
>> duplicated,
>> so this intends to address that issue.
>>
>> The goal of this first series is to address auto-generation of our
>> pack/unpack
>> functions (format_pack.c and format_unpack.c). Currently, we  have a ton
>> of
>> hand-coded pack/unpack functions for lots of formats, but we can
>> auto-generate
>> most of that code instead, so this series handles this.
>>
>> This is based on initial work by Jason Ekstrand.
>>
>> Tested on i965, classic swrast and gallium (radeon, nouveau, llvmpipe)
>> without
>> regressions.
>>
>> For software drivers we worked with a trimmed set of piglit tests
>> (related to
>> format conversion), ~5700 tests selected with the following filter:
>>
>> -t format -t color -t tex -t image -t swizzle -t clamp -t rgb -t lum -t
>> pix
>> -t fbo -t frame
>>
>> Summary of the patches:
>>   * Patches 1-7 are general fixes to the current code that were found
>> while
>>     working on this.
>>   * Patches 8-16 implement auto-generation of pack/unpack functions.
>>   * Patches 17-20 make use of the auto-generated pack/unpack functions in
>>     various places to simplify the current code.
>>
>> Notice that some of the fixes in patches 1-7 will become obsolete as soon
>> as
>> we auto-generate the pack/unpack functions, but we thought it would make
>> sense
>> to keep them in the patch set anyway since we started from that base and
>> they
>> should be correct fixes to the currently existing code.
>>
>> Iago Toral Quiroga (1):
>>    swrast: Remove unused variable.
>>
>> Jason Ekstrand (9):
>>    mesa/format_utils: Fix a bug in one of the format helper functions
>>    mesa: Fix packing/unpacking of MESA_FORMAT_R5G6B5_UNORM
>>    mesa/colormac: Remove an unused macro
>>    mesa: Fix A1R5G5B5 packing/unpacking
>>    mesa/format_utils: Prefix and expose the conversion helper functions
>>    mesa: Add a concept of an array format
>>    mesa: Add a _mesa_is_format_color_format helper
>>    mesa: Autogenerate most of format_pack.c
>>    mesa: Autogenerate format_unpack.c
>>
>> Samuel Iglesias Gonsalvez (10):
>>    mesa: Fix get_texbuffer_format().
>>    mesa: Fix _mesa_swizzle_and_convert integer conversions to clamp
>>      properly
>>    mesa: Add _mesa_pack_uint_rgba_row() format conversion function
>>    mesa: Add non-normalized formats support for ubyte packing functions
>>    mesa/format_pack: Add _mesa_pack_int_rgba_row()
>>    mesa/formats: add new mesa formats and their pack/unpack functions.
>>    mesa: use format conversion functions in swrast
>>    mesa/pack: use autogenerated format_pack functions
>>    mesa/main/pack_tmp.h: Add float conversion support
>>    mesa/pack: refactor _mesa_pack_rgba_span_float()
>>
>>   src/mesa/Makefile.am               |   18 +
>>   src/mesa/Makefile.sources          |    4 +-
>>   src/mesa/main/colormac.h           |    3 -
>>   src/mesa/main/format_convert.py    |   71 +
>>   src/mesa/main/format_info.py       |   41 +
>>   src/mesa/main/format_pack.c        | 2994 ------------------------
>>   src/mesa/main/format_pack.c.mako   | 1147 ++++++++++
>>   src/mesa/main/format_pack.h        |    6 +
>>   src/mesa/main/format_unpack.c      | 4400 ------------------------------
>> ------
>>   src/mesa/main/format_unpack.c.mako |  914 ++++++++
>>   src/mesa/main/format_utils.c       |  248 +-
>>   src/mesa/main/format_utils.h       |  105 +
>>   src/mesa/main/formats.c            |  193 +-
>>   src/mesa/main/formats.csv          |   13 +
>>   src/mesa/main/formats.h            |   73 +
>>   src/mesa/main/pack.c               | 2111 +++--------------
>>   src/mesa/main/pack_tmp.h           |   76 +-
>>   src/mesa/main/run_mako.py          |    7 +
>>   src/mesa/main/teximage.c           |    4 +-
>>   src/mesa/main/texstore.c           |    2 +-
>>   src/mesa/swrast/s_drawpix.c        |    3 -
>>   src/mesa/swrast/s_texfetch.c       |   13 +
>>   src/mesa/swrast/s_texfetch_tmp.h   | 1359 +----------
>>   23 files changed, 3222 insertions(+), 10583 deletions(-)
>>   create mode 100644 src/mesa/main/format_convert.py
>>   delete mode 100644 src/mesa/main/format_pack.c
>>   create mode 100644 src/mesa/main/format_pack.c.mako
>>   delete mode 100644 src/mesa/main/format_unpack.c
>>   create mode 100644 src/mesa/main/format_unpack.c.mako
>>   create mode 100644 src/mesa/main/run_mako.py
>>
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141118/00307be0/attachment-0001.html>


More information about the mesa-dev mailing list