On 04.05.2017 11:04, Samuel Pitoiset wrote:
The TGSI DCE pass doesn't eliminate dead assignments like
MOV TEMP[0], TEMP[1] in presence of loops because it assumes
that the visitor doesn't emit dead code. This assumption is
actually wrong and this situation happens.
However, it appears that the merge_registers() pass accidentally
takes care of this for some weird reasons. But since this pass has
been disabled for RadeonSI and Nouveau, the renumber_registers()
pass which is called *after*, can't do its job correctly.
This is because it assumes that no dead code is present. But if
there is still a dead assignment, it might re-use the TEMP
register id incorrectly and emits wrong code.
This patches fixes the issue by recording writes instead of reads,
and this has the advantage to be faster.
This should fix Unigine Heaven on RadeonSI and Nouveau.
shader-db results with RadeonSI:
47109 shaders in 29632 tests
Totals:
SGPRS: 1923308 -> 1923316 (0.00 %)
VGPRS: 1133843 -> 1133847 (0.00 %)
Spilled SGPRs: 2516 -> 2518 (0.08 %)
Spilled VGPRs: 65 -> 65 (0.00 %)
Private memory VGPRs: 1184 -> 1184 (0.00 %)
Scratch size: 1308 -> 1308 (0.00 %) dwords per thread
Code Size: 60095968 -> 60096256 (0.00 %) bytes
LDS: 1077 -> 1077 (0.00 %) blocks
Max Waves: 431889 -> 431889 (0.00 %)
Wait states: 0 -> 0 (0.00 %)
It's still interesting to disable the merge_registers() pass.
Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>
---
src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 39 ++++++++++++++++++++++++++----
1 file changed, 34 insertions(+), 5 deletions(-)
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 249584d75e..c7ed599a67 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -559,6 +559,7 @@ public:
void rename_temp_registers(int num_renames, struct rename_reg_pair
*renames);
void get_first_temp_read(int *first_reads);
+ void get_first_temp_write(int *first_writes);
void get_last_temp_read_first_temp_write(int *last_reads, int
*first_writes);
void get_last_temp_write(int *last_writes);
@@ -4759,6 +4760,33 @@ glsl_to_tgsi_visitor::rename_temp_registers(int
num_renames, struct rename_reg_p
}
void
+glsl_to_tgsi_visitor::get_first_temp_write(int *first_writes)
+{
+ int depth = 0; /* loop depth */
+ int loop_start = -1; /* index of the first active BGNLOOP (if any) */
+ unsigned i = 0, j;
+
+ foreach_in_list(glsl_to_tgsi_instruction, inst, &this->instructions) {
+ for (j = 0; j < num_inst_dst_regs(inst); j++) {
+ if (inst->dst[j].file == PROGRAM_TEMPORARY) {
+ if (first_writes[inst->dst[j].index] == -1)
+ first_writes[inst->dst[j].index] = (depth == 0) ? i :
loop_start;
+ }
+ }
+
+ if (inst->op == TGSI_OPCODE_BGNLOOP) {
+ if(depth++ == 0)
+ loop_start = i;
+ } else if (inst->op == TGSI_OPCODE_ENDLOOP) {
+ if (--depth == 0)
+ loop_start = -1;
+ }
+ assert(depth >= 0);
+ i++;
+ }
+}
+
+void
glsl_to_tgsi_visitor::get_first_temp_read(int *first_reads)
{
int depth = 0; /* loop depth */
@@ -5347,16 +5375,17 @@ glsl_to_tgsi_visitor::renumber_registers(void)
{
int i = 0;
int new_index = 0;
- int *first_reads = rzalloc_array(mem_ctx, int, this->next_temp);
+ int *first_writes = rzalloc_array(mem_ctx, int, this->next_temp);
Since the array is initialized later anyway, you don't need rzalloc here.
The only possibility I see for something going wrong here is when we
have a temporary that is read from but not written to. That would
already lead to undefined behavior today, so... with the ralloc change:
Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com>
struct rename_reg_pair *renames = rzalloc_array(mem_ctx, struct
rename_reg_pair, this->next_temp);
int num_renames = 0;
+
for (i = 0; i < this->next_temp; i++) {
- first_reads[i] = -1;
+ first_writes[i] = -1;
}
- get_first_temp_read(first_reads);
+ get_first_temp_write(first_writes);
for (i = 0; i < this->next_temp; i++) {
- if (first_reads[i] < 0) continue;
+ if (first_writes[i] < 0) continue;
if (i != new_index) {
renames[num_renames].old_reg = i;
renames[num_renames].new_reg = new_index;
@@ -5368,7 +5397,7 @@ glsl_to_tgsi_visitor::renumber_registers(void)
rename_temp_registers(num_renames, renames);
this->next_temp = new_index;
ralloc_free(renames);
- ralloc_free(first_reads);
+ ralloc_free(first_writes);
}
/* ------------------------- TGSI conversion stuff --------------------------
*/
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
