[Mesa-dev] [PATCH v6 0/6] mesa/st: glsl_to_tgsi: improved temp-reg lifetime estimation

[Nicolai Hähnle]

Hi Gert,

Thank you for the thorough update, and sorry for taking so long to 
finally review this.

I still think there are some small parts that can be removed to simplify 
things, and I've pointed those out in a reply to the main patch. But 
mostly this looks much better now, and most of my comments are just 
about trying to keep a consistent style.


On 04.07.2017 16:18, Gert Wollny wrote:
> Dear Nicolai,
> 
> this new version of the patch set that should address all the comments you gave
> for v5.
> 
> Changes are:
> 
> - 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 differently,
> - 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.
> 
> In addition to your comments I applied these changes:
> 
> - 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.

I like it.


> - 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 (Date: Fri Jun 30 10:39:53 2017)
> 
> The patches are also in
>    https://github.com/gerddie/mesa/tree/regrename-v6
> 
> Additional notes:
> * According to a user report against v5, the patch fixes #99349
> * The new register merge code doesn't have a measurable effect on the all-over
>    run-time of the shader-db.
> * There is actually not a single shader in the shader-db that requires
>    per-component tracking of the temporaries, but I achieved writing a shader
>    that really needs it and I think I will submit this to piglit.

Please do!

Thanks,
Nicolai


> 
> best regards,
> Gert
> 
> 
> 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         |  344 +----
>   src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp |  196 +++
>   src/mesa/state_tracker/st_glsl_to_tgsi_private.h   |  167 +++
>   .../state_tracker/st_glsl_to_tgsi_temprename.cpp   | 1066 ++++++++++++++
>   .../state_tracker/st_glsl_to_tgsi_temprename.h     |   67 +
>   src/mesa/state_tracker/tests/Makefile.am           |   36 +
>   .../tests/test_glsl_to_tgsi_lifetime.cpp           | 1556 ++++++++++++++++++++
>   10 files changed, 3106 insertions(+), 333 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
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.