[Mesa-dev] [PATCH v9 0/7] mesa/st: glsl_to_tgsi: refined register merge algorithm
Gert Wollny
gw.fossdev at gmail.com
Wed Sep 6 10:42:00 UTC 2017
Am Mittwoch, den 06.09.2017, 11:53 +0200 schrieb Nicolai Hähnle:
> I finally went over this again, and I think it's good enough to go
> in. R-b and pushed!
Thanks :)
Unfortunately it fails to build on this AppVoyager scons build (MS
Windows?)
The first failure I see is that it doesn't recognize
numeric_limits<int>::max()
even though <limits> is included and
using std::numeric_limits;
specified.
The other problem is that it doesn't recognize "access_record", maybe
the compiler expects "struct" to be specified every time (like in C)?
> I did notice some minor things, I'm about to send out patches to
> clean things up afterwards.
I will test these patches ASAP.
Best,
Gert
>
> Cheers,
> Nicolai
>
>
> On 24.08.2017 19:38, Gert Wollny wrote:
> > Dear all,
> >
> > I thought I might send out this patch another time with its full
> > history and
> > freshly rebased. All the changes that I applied were a result of
> > reviews by
> > Nicolai (mostly) and Emil (thanks again to both of you).
> >
> > The set is mirroed at
> > https://github.com/gerddie/mesa/tree/regrename-v9
> >
> > The patch fixes a series of bugs where shader compilation fails
> > with
> > "translation from TGSI failed!"
> > Among these are
> > * https://bugs.freedesktop.org/show_bug.cgi?id=65448 which
> > I can confirm will be fixed for R600_DEBUG=nosb set (with sb
> > enabled it will
> > fail with a failing assertion in the sb code).
> > * According to a user report against v5, the patch also fixes
> > #99349
> >
> > I can also confirm that the patch fixes the Piano and Voloplosion
> > benchmarks
> > implemented in gputest on BARTS (r600g).
> >
> > The patch has no significant impact on runtime - not taking Dave's
> > patch into
> > account that in itself reduces the register renaming run-time for
> > shaders
> > with a large numbers of temporary registers.
> >
> > The patch doesn't introduce piglit regression (I tested the shader
> > subset).
> > spec at glsl-1.50@execution at variable-indexing@gs-input-array-vec2-
> > index-rd is
> > fixed though.
> >
> > The algorithm works like follows:
> > - first the program is scanned, the loops, switch and if/else
> > scopes are
> > collected and for each temporary first and last reads and writes
> > and the
> > according scopes are collected, and it is recorded whether a
> > variable is
> > written conditionally, and whether loops have continue or break
> > statements.
> > - then after the whole program has been scanned, the life times are
> > estimated
> > by merging the read and write scopes for each temporary on a per
> > component
> > bases,
> > - the life-times of the cmponents are merged,
> > - the register mapping is evaluated, and
> > - the mapping is applied with the rename_temp_registers method
> > implemented
> > by Dave.
> >
> > I've used the patches for quite some time now and so far I didn't
> > encounter
> > any problems, many thanks for any comments,
> >
> > Gert
> >
> > Patch history:
> >
> > v2:* significantly cut down on the memory allocations,
> > * expose only a minimal interface to register lifetime
> > estimation and
> > calculating the rename table,
> >
> > v3: was broken and v4 restarted from v2
> >
> > v4:* split the changes into more patches
> > * correct formatting errors,
> > * remove the use of the STL with one exception though:
> > since in st_glsl_to_tgsi.cpp std::sort is already used and
> > its run-time
> > performance is significantly better than qsort. It is used in
> > the register
> > rename mapping evaluation. It can be disabled by commenting
> > out the define
> > USE_STL_SORT in st_glsl_to_tgsi_temprename.cpp.
> > * add more tests and improve the life-time evaluation
> > accordingly,
> > * further reduce memory allocations,
> > * rename functions and methods to better clarify what they are
> > used for,
> > * remove unused methods and variables in prog_scope,
> > * eliminate the class tgsi_temp_lifetime,
> > * no longer require C++11 for the core library code, however,
> > the tests
> > make use of C++11 and the STL
> >
> > v5: * correct formatting following Emil's suggetions
> > * remove un-needed libraries for the tests
> >
> > v6:* the components are now tracked individually and the life time
> > of a temporary
> > is evaluated by merging the life-times of their components,
> > * BRK/CONT are now handled separately,
> > * the final algorithm to evaluate the life-times was
> > simplified,
> > * read and write in the same instruction is now considered to
> > be always
> > well defined,
> > * adherence to the coding stile was improved,
> > * the case scope level is now below the according switch scope
> > level,
> > * the new register merge method replaces the old version, i.e.
> > no environment
> > variables to switch between implementations. In theory, one
> > could also
> > remove the function get_last_temp_read_first_temp_write, but
> > is is still
> > used in some code in a #define 0 block, so I didn't touch it.
> > * when compiled in debug mode and with the environment variable
> > GLSL_TO_TGSI_RENAME_DEBUG specified the TGSI and resulting
> > register
> > lifetimes will be dumped to stderr.Here Nicolai suggested to
> > use
> > _mesa_register_file_name instead of my hand-backed array of
> > strings,
> > but currently that function doesn't write out all the needed
> > names,
> > so I thought it might be better to address this in another
> > patch that
> > also extends _mesa_register_file_name,
> > * unused registers are now ignored in the rename mapping
> > evaluation,
> > * registers that are only read get a life-time {x,x}, with x
> > the instruction
> > line were the register is last read, so they can be merged,
> > * the patch has been rebased against 7d7bcd65d
> >
> > v7:* Correct documentation of GLSL_TO_TGSI_RENAME_DEBUG in commit
> > message.
> > * fix typos, include files, formatting, and some variable
> > names,
> > * add anonymous namespace around classes,
> > * replace debug_log singleton by a function,
> > * cleanup the switch-case scope creation,
> > * track switch variable only for SWITCH statement,
> > * add test case with switch in loop that has a case where the
> > register is
> > written, then falls through where it is read and correct code
> > accordingly,
> > specifically, this showed that Nicolai was right that the
> > scope of the
> > case that falls through must not be extended,
> > * add test case for more then one break in loop and make sure
> > the first break
> > is used for life-time tracking,
> > * simplify access flag tracking and merging,
> > * drop special handling of registers that are only read, i.e.
> > treat them like
> > unused registers since renumber_registers will do this
> > anyway,
> > * simplify the enclosing scope resolution,
> > * handle TEMP[0] just like the others; somehow I had the idea
> > that TEMP[0] is
> > special and should not be touched, and the debug output
> > running various
> > programs and also from piglit shaders doesn't show TEMP[0] as
> > used,
> > * optimize renaming evaluation by only copying used registers
> > to the
> > reg_access array that is used for the evaluation,
> >
> >
> > v8:* added Dave Airlie's patch for register renaming slightly
> > changed to
> > account for the inst->resource_index since the reminder of
> > the patch set
> > makes use of his renaming methods. Since my mapping evaluator
> > doesn't
> > require a recursive renaming strategy, the problem that made
> > Dave
> > revert this patch does not occure after applying the series,
> >
> > v9:* add check to handle op-codes TGSI_OPCODE_CAL and
> > TGSI_OPCODE_RET, i.e.
> > if these opcondes are encountered that the register lifetime
> > estimation
> > will bailout and signal that no renaming can take place
> > (because subroutines
> > are not tracked),
> > * add array-id to debug output for PROGRAM_ARRAY,
> > * rebase to 39f3e2507c6
> >
> > Dave Airlie (1):
> > st_glsl_to_tgsi: rewrite rename registers to use array fully.
> >
> > Gert Wollny (6):
> > mesa/st: glsl_to_tgsi move some helper classes to extra files
> > mesa/st: glsl_to_tgsi: implement new temporary register lifetime
> > tracker
> > mesa/st: glsl_to_tgsi: add tests for the new temporary lifetime
> > tracker
> > mesa/st: glsl_to_tgsi: add register rename mapping evaluator
> > mesa/st: glsl_to_tgsi: Add test set for evaluation of rename
> > mapping
> > mesa/st: glsl_to_tgsi: tie in new temporary register merge
> > approach
> >
> > configure.ac | 1 +
> > src/mesa/Makefile.am | 2 +-
> > src/mesa/Makefile.sources | 4 +
> > src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 400 +----
> > src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp | 196 +++
> > src/mesa/state_tracker/st_glsl_to_tgsi_private.h | 168 ++
> > .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 1008
> > ++++++++++++
> > .../state_tracker/st_glsl_to_tgsi_temprename.h | 71 +
> > src/mesa/state_tracker/tests/Makefile.am | 36 +
> > .../tests/test_glsl_to_tgsi_lifetime.cpp | 1600
> > ++++++++++++++++++++
> > 10 files changed, 3124 insertions(+), 362 deletions(-)
> > create mode 100644
> > src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp
> > create mode 100644
> > src/mesa/state_tracker/st_glsl_to_tgsi_private.h
> > create mode 100644
> > src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
> > create mode 100644
> > src/mesa/state_tracker/st_glsl_to_tgsi_temprename.h
> > create mode 100644 src/mesa/state_tracker/tests/Makefile.am
> > create mode 100644
> > src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp
> >
>
>
More information about the mesa-dev
mailing list