[Mesa-dev] GLSL built-in functions rewrite

[Paul Berry]

On 4 September 2013 15:22, Kenneth Graunke <kenneth at whitecape.org> wrote:

> Hello!
>
> This series (at long last!) rewrites the GLSL built-in function handling,
> for the third time.  It's available on the builtins-v3 branch of ~kwg/mesa.
>
> The existing built-in function code had a lot of drawbacks:
>
> 1. Writing new built-ins in the S-Expression style IR was painful:
>
>    Something as simple as x^2 + 1 turned into:
>
>    (expression vec4 + (expression vec4 * (var_ref x) (var_ref x))
>                       (constant float (1)))
>
>    which is extremely wordy, and makes it difficult to follow the
>    algorithm.  IR builder's add(mul(x, x), imm(1.0f)) is much nicer.
>
> 2. Writing new built-ins in GLSL kind of worked some of the time:
>
>    To combat the above, we added support for writing built-ins in GLSL.
>    Unfortunately, they couldn't reliably use other built-in functions,
>    including essentials like dot(), so this approach was pretty limited.
>
> 3. Myriads of cut and pasted prototypes:
>
>    Adding support for a new GLSL version meant cut and pasting the previous
>    version's prototypes, and adjusting a few tiny things.  This resulted in
>    thousands of duplicated lines, which as we support more and more GLSL
>    versions means propagating bug fixes is a nightmare.
>
> 4. Poor control over built-in availability:
>
>    Whether foo() is available depends on the language version, shader
> stage,
>    and enabled extensions...in some arbitrary combination.  The old system
>    worked for simple constraints, but didn't adequately handle things like
>    "extensions A /and/ B enabled, or extension A and shading language X..."
>
> 5. Build system nightmare.
>
>    First, it had to build builtin_compiler for the host architecture.
>  Next,
>    it would invoke Python scripts to read IR files, invoke
> builtin_compiler,
>    and generate a several megabyte C++ source file containing giant strings
>    of S-Expressions.  Then it had to recompile the compiler with the actual
>    built-in functions, this time for the target architecture.  When not
>    cross-compiling, it would skip some of the recompiling.
>
>    We had to make all that work with Autotools, SCons, and Android.
>
>    We broke cross-compiling so many times.
>
>    When you inevitably broke the GLSL compiler, you couldn't even compile
>    Mesa (the built-ins wouldn't compile), making debugging a challenge.
>
> 6. Slow startup time.
>
>    When requesting a built-in function, we had to parse the strings into
>    S-Expressions, then parse those to generate actual IR.  This took a
> while.
>
> 7. Finding a matching built-in was complicated.
>
>    Built-ins were spread across multiple gl_shader objects, so the code to
>    find a matching signature had to loop over all of them.  Worse, one of
>    the early shaders might define an inexact match, while a later one had
>    an exact match.  Multiple passes, huzzah!
>
> But I digress.
>
> This series takes a significantly different approach: it implements the
> built-in functions directly in C++, using ir_builder.  All signatures are
> stored in a single shader object.  Availability is expressed via boolean
> predicates, which can be arbitrary expressions using _mesa_glsl_parse_state
> fields.  Unavailable functions are simply filtered out when looking for
> matching built-ins and importing prototypes.
>
> It's significantly less code:
>
>    166 files changed, 3774 insertions(+), 11918 deletions(-)
>
>    (we could possibly scrap more...the S-Expression code and IR reader
>     still exist for unit testing purposes.)
>
> It's also faster:
>
>    $ piglit-run.py -t glslparsertest tests/quick.tests r
>
> takes about 12% less time.
>
> There's no crazy two-stage compilation process: no builtin_compiler,
> no Python scripts, no cross compiling headaches.
>
> I'd appreciate any feedback you might have.  Testing with non-Autotools
> would be appreciated as well.
>
> I found no Piglit or Khronos ES3 conformance regressions on i965/IVB.
>
> --Ken
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>

I sent comments on patches 1, 4, 5, and 18.  Patches 20 and 21 are:

Acked-by: Paul Berry <stereotype441 at gmail.com>

(20 because I'm not an expert on Mesa's build system, 21 because I didn't
delve into detail to make sure you had deleted exactly the right set of
files).

The remainder are:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>

My one major reservation about the series is the texturing functions.  I
doubt anyone is going to review them carefully, and we know that there is
woefully inadequate piglit testing for texturing.  But I still think it's
worth moving forward with this series because:

(a) it's such a tremendous improvement on what we have now that I'm willing
to put up with the risk of introducing some texturing bugs for the sake of
this much code cleanup.

(b) we are at a good point in the release cycle to make a change like
this--there is a lot of time for texturing bugs to be discovered and fixed.

(c) if we do discover texturing bugs between now and the next release, that
will be a good opportunity to improve our piglit testing of texturing
functions.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130908/1853e040/attachment.html>