<div dir="ltr">On 4 September 2013 15:22, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div><div class="gmail_extra">I sent comments on patches 1, 4, 5, and 18. Patches 20 and 21 are: Acked-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div><div class="gmail_extra">(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 <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div><div class="gmail_extra">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: </div><div class="gmail_extra">(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. </div><div class="gmail_extra"> (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. </div><div class="gmail_extra">(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. </div></div>