Mesa (staging/20.3): nir: Redefine start/end_ip of blocks to fix NIR-to-TGSI liveness bugs.
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Mon Dec 14 17:32:27 UTC 2020
Module: Mesa
Branch: staging/20.3
Commit: f9bc767bdc5eb7cfa2fc2188beae7f5729ec0587
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=f9bc767bdc5eb7cfa2fc2188beae7f5729ec0587
Author: Eric Anholt <eric at anholt.net>
Date: Fri Dec 11 09:48:15 2020 -0800
nir: Redefine start/end_ip of blocks to fix NIR-to-TGSI liveness bugs.
With the block's end_ip accidentally being the ip of the next instruction,
contrary to the comment, you would end up doing end-of-block freeing early
and have the value missing when it came time to emit the next instruction.
Just expand the ips to have separate ones for start and end of block --
while it means that nir_instr->index is no longer incremented by 1 per
instruction, it makes sense for use in liveness because a backend is
likely to need to do other things at block boundaries (like emit the if
statement's code), and having an ip to identify that stuff is useful.
Fixes: a206b581578d ("nir: Add a block start/end ip to live instr index metadata.")
Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7658>
(cherry picked from commit d3d28f6c2d1795d391442249343d8cd38356665d)
---
.pick_status.json | 2 +-
src/compiler/nir/nir.c | 4 ++--
src/compiler/nir/nir.h | 10 ++++------
src/compiler/nir/nir_liveness.c | 14 +++++---------
src/gallium/auxiliary/nir/nir_to_tgsi.c | 14 +++++++++++++-
5 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/.pick_status.json b/.pick_status.json
index 52ec3cab69d..1d545baa24c 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -220,7 +220,7 @@
"description": "nir: Redefine start/end_ip of blocks to fix NIR-to-TGSI liveness bugs.",
"nominated": true,
"nomination_type": 1,
- "resolution": 0,
+ "resolution": 1,
"master_sha": null,
"because_sha": "a206b581578d585d845250f62dfb1e6684ddf2f0"
},
diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
index acedb0536ab..2d1478f152b 100644
--- a/src/compiler/nir/nir.c
+++ b/src/compiler/nir/nir.c
@@ -1945,12 +1945,12 @@ nir_index_instrs(nir_function_impl *impl)
unsigned index = 0;
nir_foreach_block(block, impl) {
- block->start_ip = index;
+ block->start_ip = index++;
nir_foreach_instr(instr, block)
instr->index = index++;
- block->end_ip = index;
+ block->end_ip = index++;
}
return index;
diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 9365c163773..3979b6b4e2e 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -2796,15 +2796,13 @@ typedef struct nir_block {
uint32_t dom_pre_index, dom_post_index;
/**
- * nir_instr->index for the first nir_instr in the block. If the block is
- * empty, it will be the index of the immediately previous instr, or 0.
- * Valid when the impl has nir_metadata_instr_index.
+ * Value just before the first nir_instr->index in the block, but after
+ * end_ip that of any predecessor block.
*/
uint32_t start_ip;
/**
- * nir_instr->index for the last nir_instr in the block. If the block is
- * empty, it will be the same as start_ip. Valid when the impl has
- * nir_metadata_instr_index.
+ * Value just after the last nir_instr->index in the block, but before the
+ * start_ip of any successor block.
*/
uint32_t end_ip;
diff --git a/src/compiler/nir/nir_liveness.c b/src/compiler/nir/nir_liveness.c
index 8ed6d844ba1..04a00cbc180 100644
--- a/src/compiler/nir/nir_liveness.c
+++ b/src/compiler/nir/nir_liveness.c
@@ -326,36 +326,32 @@ nir_live_ssa_defs_per_instr(nir_function_impl *impl)
for (int i = 0; i < impl->ssa_alloc; i++)
liveness->defs->start = ~0;
- unsigned last_instr = 0;
nir_foreach_block(block, impl) {
unsigned index;
BITSET_FOREACH_SET(index, block->live_in, impl->ssa_alloc) {
liveness->defs[index].start = MIN2(liveness->defs[index].start,
- last_instr);
+ block->start_ip);
}
nir_foreach_instr(instr, block) {
nir_foreach_ssa_def(instr, def_cb, liveness);
-
- last_instr = instr->index;
};
/* track an if src's use. We need to make sure that our value is live
* across the if reference, where we don't have an instr->index
- * representing the use. Mark it as live through the next real
- * instruction.
+ * representing the use. Mark it as live through the end of the block.
*/
nir_if *nif = nir_block_get_following_if(block);
if (nif) {
if (nif->condition.is_ssa) {
liveness->defs[nif->condition.ssa->index].end = MAX2(
- liveness->defs[nif->condition.ssa->index].end,
- last_instr + 1);
+ liveness->defs[nif->condition.ssa->index].end, block->end_ip);
}
}
BITSET_FOREACH_SET(index, block->live_out, impl->ssa_alloc) {
- liveness->defs[index].end = MAX2(liveness->defs[index].end, last_instr);
+ liveness->defs[index].end = MAX2(liveness->defs[index].end,
+ block->end_ip);
}
}
diff --git a/src/gallium/auxiliary/nir/nir_to_tgsi.c b/src/gallium/auxiliary/nir/nir_to_tgsi.c
index d74eafd06a3..7d23ab08bce 100644
--- a/src/gallium/auxiliary/nir/nir_to_tgsi.c
+++ b/src/gallium/auxiliary/nir/nir_to_tgsi.c
@@ -48,6 +48,9 @@ struct ntt_compile {
unsigned loop_label;
+ /* if condition set up at the end of a block, for ntt_emit_if(). */
+ struct ureg_src if_cond;
+
/* TGSI temps for our NIR SSA and register values. */
struct ureg_dst *reg_temp;
struct ureg_dst *ssa_temp;
@@ -2046,7 +2049,7 @@ static void
ntt_emit_if(struct ntt_compile *c, nir_if *if_stmt)
{
unsigned label;
- ureg_UIF(c->ureg, ntt_get_src(c, if_stmt->condition), &label);
+ ureg_UIF(c->ureg, c->if_cond, &label);
ntt_emit_cf_list(c, &if_stmt->then_list);
if (!exec_list_is_empty(&if_stmt->else_list)) {
@@ -2116,6 +2119,15 @@ ntt_emit_block(struct ntt_compile *c, nir_block *block)
nir_foreach_src(instr, ntt_src_live_interval_end_cb, c);
}
+ /* Set up the if condition for ntt_emit_if(), which we have to do before
+ * freeing up the temps (the "if" is treated as inside the block for liveness
+ * purposes, despite not being an instruction)
+ */
+ nir_if *nif = nir_block_get_following_if(block);
+ if (nif)
+ c->if_cond = ntt_get_src(c, nif->condition);
+
+ /* Free up any SSA temps that are unused at the end of the block. */
unsigned index;
BITSET_FOREACH_SET(index, block->live_out, BITSET_WORDS(c->impl->ssa_alloc)) {
unsigned def_end_ip = c->liveness->defs[index].end;
More information about the mesa-commit
mailing list