Dear all, I'd like to get this series in the 18.3 release cycle applied. I've been using it for quite some time without issues, and it seems to fix a number of problems on r600. For instance, a number of dEQP-GLES31 tests fail because of issued with spilling, but with these patches the need for spilling can be eliminated in these cases. I also didn't see any piglit regressions, and the patches still apply cleanly.
Many thanks to anyone who reviews, or at least ack. this series, Gert Am Dienstag, den 05.06.2018, 22:26 +0200 schrieb Gert Wollny: > taking the comments of Nicolai Hähnle into account I've updated the > series. I'd also > like to thank Benedikt Schemmer for going out on the adventure to > these these patches > on radeonsi for which this code path is actually not relevant. It > made me realize, > that I was not tracking the inst->resources register. I think, > however, that r600 and > llvmpipe are not really using that register, so I cannot test whether > there are any > fixes or regressions coming with this (for me the piglit results > didn't change). > > v4: > - make the TGSI statistics collection routine thread save, > - rework the classes that are used to evaluate and apply the array > merging (I > did not follow Nicolais suggestion to handle the array live range > also within > the class that tracks temporary registers because I think the > handling is > sufficiently different, and because the temporary register live > range > evaluation code is already difficult enough, no need to add more > complexity > to also handle arrays), > - inst->resource to be also taken into account in the live range > tracking and > array remapping > - add two patches that add test infrastructure and unit tests for > array live > range tracking and remapping > > v3: > - Add new test mesa/st/tests/meson.build > - rebase patches to latest HEAD > > this is the merged version of two series [1] (TGSI: split, merge > and interleave arrays) and [2] (mesa/st/glsl_to_tgsi: Properly > resolve life times for simple if/else + use constructs) I sent > earlier. Considering that both parts target the same optimization > step and fix a bug if both are applied, I thought it is better to > add this second patch to the series. Changes refer to v1 of [1]: > > v2: > - rebase patches to latest HEAD > - add some code that allows obtaining some statistics about register > and instruction usage > - Add patch [2] that improves resolving the live range estimation > with > simple if/else and use constructs. By adding this patch the > series > fixes https://bugs.freedesktop.org/show_bug.cgi?id=105371 > > v1: > Patch 1: Split arrays that are only accessed directly: > I posted a first version off the the array splitting in patch 1 some > time ago. Eric Anholt pointed out that this might be done in > opt_array_splitting.cpp, but in another comment Timothy pointed out > that this is far from trivial, and he also pointed out that he was > proposing similar patches for NIR, but since currently no NIR->TGSI > transformation is available, TGSI based drivers can't make use of > this. > > While the reminder off the series could be applied without this > patch, I > think it makes less sense to do all the optimizations on arrays that > could > simply be split into individual registers, so I repost the patch with > some > changes. > > I tried to be exhaustive with comments and make the variable any type > names > self-explaining, but since I've been staring at this code for a long > time I > don't think I am capable of seeing any problems any more, so comments > are very > welcome. > > Many thanks for reviews and comments, > Gert > > PS: I have no commit rights. > > Gert Wollny (15): > mesa/st/glsl_to_tgsi: Add method to collect some TGSI statistics > mesa/st/glsl_to_tgsi: Split arrays whose elements are only accessed > directly > mesa/st/glsl_to_tgsi: Properly resolve life times simple if/else + > use > constructs > mesa/st/glsl_to_tgsi:rename lifetime to register_live_range > mesa/st/glsl_to_tgsi: Add helper class for array live range merging > and interleaving > mesa/st/glsl_to_tgsi: Add helper classes to apply array merging and > interleaving > mesa/st/glsl_to_tgsi: Add array merge logic > mesa/st/tests: Add tests for array merge helper classes. > mesa/st/glsl_to_tgsi: rename access_record to register_merge_record > and some more renames > mesa/st/glsl_to_tgsi: move evaluation of read mask up in the call > hierarchy > mesa/st/glsl_to_tgsi: add class for array access tracking > mesa/st/glsl_to_tgsi: add array life range evaluation into tracking > code > mesa/st/glsl_to_tgsi: Expose array live range tracking and merging > mesa/st/tests: Add array life range tests infrastructure to common > test class > mesa/st/tests: Add array life range estimation and renumbering > tests > > src/mesa/Makefile.sources | 2 + > src/mesa/meson.build | 2 + > src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 214 ++++- > .../state_tracker/st_glsl_to_tgsi_array_merge.cpp | 698 > +++++++++++++++ > .../state_tracker/st_glsl_to_tgsi_array_merge.h | 188 ++++ > .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 307 +++++-- > .../state_tracker/st_glsl_to_tgsi_temprename.h | 45 +- > src/mesa/state_tracker/tests/Makefile.am | 20 +- > src/mesa/state_tracker/tests/meson.build | 16 +- > src/mesa/state_tracker/tests/st_tests_common.cpp | 190 +++- > src/mesa/state_tracker/tests/st_tests_common.h | 52 +- > .../tests/test_glsl_to_tgsi_array_merge.cpp | 962 > +++++++++++++++++++++ > .../tests/test_glsl_to_tgsi_lifetime.cpp | 35 +- > 13 files changed, 2579 insertions(+), 152 deletions(-) > create mode 100644 > src/mesa/state_tracker/st_glsl_to_tgsi_array_merge.cpp > create mode 100644 > src/mesa/state_tracker/st_glsl_to_tgsi_array_merge.h > create mode 100644 > src/mesa/state_tracker/tests/test_glsl_to_tgsi_array_merge.cpp > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
