Mesa (main): nir: prevent peephole from generating invalid NIR

[GitLab Mirror]

Module: Mesa
Branch: main
Commit: a13e79843e054cc9b35f8f5b8b31fdf28b6ae95d
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=a13e79843e054cc9b35f8f5b8b31fdf28b6ae95d

Author: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
Date:   Mon Aug 23 16:45:20 2021 +0300

nir: prevent peephole from generating invalid NIR

We can't append instructions following a return/halt instruction
because the control flow helpers will modify the successor of the
block containing the return/halt. And the NIR validator enforces that
the return/halt must have the end of the function as successor.

This tends to happen following lower_shader_calls lowering which
inserts halts. This probably doesn't prevent the optimization, it'll
just happen in one of the return shaders after the halt has been
removed.

v2: Move prev block ending check earlier in the function (Daniel)

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
Reviewed-by: Daniel Schürmann <daniel at schuermann.dev>
Cc: mesa-stable
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12506>

---

 src/compiler/nir/nir.h                     | 15 +++++++++++++++
 src/compiler/nir/nir_opt_peephole_select.c | 13 +++++++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 04ca38fb014..c4207a27dbd 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -2938,6 +2938,21 @@ nir_block_ends_in_jump(nir_block *block)
           nir_block_last_instr(block)->type == nir_instr_type_jump;
 }
 
+static inline bool
+nir_block_ends_in_return_or_halt(nir_block *block)
+{
+   if (exec_list_is_empty(&block->instr_list))
+      return false;
+
+   nir_instr *instr = nir_block_last_instr(block);
+   if (instr->type != nir_instr_type_jump)
+      return false;
+
+   nir_jump_instr *jump_instr = nir_instr_as_jump(instr);
+   return jump_instr->type == nir_jump_return ||
+          jump_instr->type == nir_jump_halt;
+}
+
 static inline bool
 nir_block_ends_in_break(nir_block *block)
 {
diff --git a/src/compiler/nir/nir_opt_peephole_select.c b/src/compiler/nir/nir_opt_peephole_select.c
index 5eeb5f66b94..72b6249cf43 100644
--- a/src/compiler/nir/nir_opt_peephole_select.c
+++ b/src/compiler/nir/nir_opt_peephole_select.c
@@ -381,6 +381,17 @@ nir_opt_peephole_select_block(nir_block *block, nir_shader *shader,
    if (prev_node->type != nir_cf_node_if)
       return false;
 
+   nir_block *prev_block = nir_cf_node_as_block(nir_cf_node_prev(prev_node));
+
+   /* If the last instruction before this if/else block is a jump, we can't
+    * append stuff after it because it would break a bunch of assumption about
+    * control flow (nir_validate expects the successor of a return/halt jump
+    * to be the end of the function, which might not match the successor of
+    * the if/else blocks).
+    */
+   if (nir_block_ends_in_return_or_halt(prev_block))
+      return false;
+
    nir_if *if_stmt = nir_cf_node_as_if(prev_node);
 
    /* first, try to collapse the if */
@@ -422,8 +433,6 @@ nir_opt_peephole_select_block(nir_block *block, nir_shader *shader,
     * selects.
     */
 
-   nir_block *prev_block = nir_cf_node_as_block(nir_cf_node_prev(prev_node));
-
    /* First, we move the remaining instructions from the blocks to the
     * block before.  We have already guaranteed that this is safe by
     * calling block_check_for_allowed_instrs()