Mesa (staging/21.2): nir: prevent peephole from generating invalid NIR

[GitLab Mirror]

Module: Mesa
Branch: staging/21.2
Commit: 71a2d809806e5d584aeba8105aaabd17a5687052
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=71a2d809806e5d584aeba8105aaabd17a5687052

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>
(cherry picked from commit a13e79843e054cc9b35f8f5b8b31fdf28b6ae95d)

---

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

diff --git a/.pick_status.json b/.pick_status.json
index 58ad650db7c..434cbb74585 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -1444,7 +1444,7 @@
         "description": "nir: prevent peephole from generating invalid NIR",
         "nominated": true,
         "nomination_type": 0,
-        "resolution": 0,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": null
     },
diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index efeaff2d258..dd6cb11094e 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -2749,6 +2749,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()