[Mesa-dev] [PATCH 0/9][RFC] GLSL preprocessor/parser improvements

Mon May 22 01:41:45 UTC 2017

I suspect you forgot to test with a debug build, there is issues with 
the string buffer  changes somewhere.

ralloc.c:203: reralloc_size: Assertion `ralloc_parent(ptr) == ctx' failed.

#0  0x00007ffff70eb91f in raise () from /lib64/libc.so.6
#1  0x00007ffff70ed51a in abort () from /lib64/libc.so.6
#2  0x00007ffff70e3da7 in __assert_fail_base () from /lib64/libc.so.6
#3  0x00007ffff70e3e52 in __assert_fail () from /lib64/libc.so.6
#4  0x00007ffff442c73c in reralloc_size (ctx=ctx at entry=0x7fffd40cb910, 
ptr=<optimized out>, size=<optimized out>) at ralloc.c:203
#5  0x00007ffff44b519e in glcpp_preprocess 
(ralloc_ctx=ralloc_ctx at entry=0x7fffd40c9da0, 
shader=shader at entry=0x7fffea7cfcc8, info_log=info_log at entry=0x7fffd40ca058,
     extensions=extensions at entry=0x7ffff43ff070 
<add_builtin_defines(_mesa_glsl_parse_state*, void (*)(glcpp_parser*, 
char const*, int), glcpp_parser*, unsigned int, bool)>, 
state=state at entry=0x7fffd40c9da0,
     gl_ctx=gl_ctx at entry=0x7fffe85c7010) at glsl/glcpp/pp.c:246
#6  0x00007ffff4402665 in _mesa_glsl_compile_shader 
(ctx=ctx at entry=0x7fffe85c7010, shader=shader at entry=0x7fffd40c8cc0, 
dump_ast=dump_ast at entry=false, dump_hir=dump_hir at entry=false, 
force_recompile=force_recompile at entry=false)
     at glsl/glsl_parser_extras.cpp:2065

On 22/05/17 06:49, Thomas Helland wrote:
> This patch series contains some of the work done by Vladislav
> in the beginning of March, that seems to have been forgotten.
> 
> For reference, that series, with review comments, can be found here:
> https://lists.freedesktop.org/archives/mesa-dev/2017-January/139892.html
> 
> I've adressed some of the review comments, most notably redoing the
> string buffer implementation quite a bit, and adding tests.
> The major change is that it is now decoupled from the parser
> and instead lives in /src/util/ as a utility for easier reuse.
> I've also closely followed the structure of our hash table and set
> so that it should be quite familiar for everyone. My implementation
> is also null terminated, while Vladislav's implementation was not.
> My reasoning behind this was that it's less fragile if someone where
> to access the contents of the buffer directly. I've also expanded
> with some different functions that allow us to do more stuff.
> Obviously this could be expanded upon further if need be.
> 
> I've included some of Vladislav's less involved patches that
> I think we should get in as soon as possible. I've added Ian's
> r-b on the ones that he reviewed (I hope that's OK). The one
> patch that is missing from this series is the hand-written
> custom parser. While this was the thing that gave the biggest
> speed-up, unfortunately it's also a bit harder to review,
> so I've left that as a future exercise.
> 
> I've run it through my complete shader-db and there's no changes
> or breakages, so that's encouraging. It amounts to about a 3%
> reduction in runtime and executed instructions on the shader-db run.
> It should be noted that the i965 backend is the primary bottleneck
> when running shader-db on my computer, so the numbers don't look all
> that impressive due to this. I'll see if I can come up with some
> numbers using the gallium noop driver.
> 
> I have not done a very thorough job on testing that the output
> of the preprocessor/parser is the same after this series.
> However, there's no changes here that I believe should cause
> any issues, so I feel quite confident that this series should
> not cause any trouble. The original series did have some issues
> discovered by running a fuzzer over it, however I expect that
> was caused by the introduced hand-written fast-path scanner and
> associated macro substitutaion that are not a part of this series.
> 
> CC: Vladislav Egorov <vegorov180 at gmail.com>
> CC: Ian Romanick <idr at freedesktop.com>
> 
> Thomas Helland (5):
>    util: Add a string buffer implementation
>    util: Add tests for the string buffer
>    glsl: Change the parser to use the string buffer
>    glcpp: Use string_buffer for line continuation removal
>    glcpp: Avoid unnecessary call to strlen
> 
> Vladislav Egorov (4):
>    glcpp: Avoid unnecessary strcmp()
>    glcpp: Skip unnecessary line continuations removal
>    ralloc: Use strnlen() inside of strncat()
>    glcpp: Use Bloom filter before identifier search
> 
>   configure.ac                                    |   1 +
>   src/compiler/glsl/glcpp/glcpp-lex.l             |   3 +-
>   src/compiler/glsl/glcpp/glcpp-parse.y           | 123 +++++++++-------
>   src/compiler/glsl/glcpp/glcpp.h                 |  18 ++-
>   src/compiler/glsl/glcpp/pp.c                    |  73 ++++++----
>   src/util/Makefile.am                            |   3 +-
>   src/util/Makefile.sources                       |   2 +
>   src/util/ralloc.c                               |   7 +-
>   src/util/string_buffer.c                        | 180 ++++++++++++++++++++++++
>   src/util/string_buffer.h                        |  75 ++++++++++
>   src/util/tests/string_buffer/Makefile.am        |  34 +++++
>   src/util/tests/string_buffer/append_and_print.c |  82 +++++++++++
>   12 files changed, 505 insertions(+), 96 deletions(-)
>   create mode 100644 src/util/string_buffer.c
>   create mode 100644 src/util/string_buffer.h
>   create mode 100644 src/util/tests/string_buffer/Makefile.am
>   create mode 100644 src/util/tests/string_buffer/append_and_print.c
> 