2017-05-22 3:41 GMT+02:00 Timothy Arceri <tarc...@itsqueeze.com>: > 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@entry=0x7fffd40cb910, > ptr=<optimized out>, size=<optimized out>) at ralloc.c:203 > #5 0x00007ffff44b519e in glcpp_preprocess > (ralloc_ctx=ralloc_ctx@entry=0x7fffd40c9da0, > shader=shader@entry=0x7fffea7cfcc8, info_log=info_log@entry=0x7fffd40ca058, > extensions=extensions@entry=0x7ffff43ff070 > <add_builtin_defines(_mesa_glsl_parse_state*, void (*)(glcpp_parser*, char > const*, int), glcpp_parser*, unsigned int, bool)>, > state=state@entry=0x7fffd40c9da0, > gl_ctx=gl_ctx@entry=0x7fffe85c7010) at glsl/glcpp/pp.c:246 > #6 0x00007ffff4402665 in _mesa_glsl_compile_shader > (ctx=ctx@entry=0x7fffe85c7010, shader=shader@entry=0x7fffd40c8cc0, > dump_ast=dump_ast@entry=false, dump_hir=dump_hir@entry=false, > force_recompile=force_recompile@entry=false) > at glsl/glsl_parser_extras.cpp:2065 > >
Your suspicion is of course correct. Switching back and forth between profiling and coding and I forgot to switch debug back on. I'll look into this, plus addressing all of the nice feedback. Thanks for taking a look. > > 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 <vegorov...@gmail.com> >> CC: Ian Romanick <i...@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 >> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
