[Mesa-dev] [PATCH v3 2/2] nir/dead_cf: also remove useless ifs

[Caio Marcelo de Oliveira Filho]

Generalize the code for remove dead loops to also remove dead if
nodes. The conditions are the same in both cases, if the node (and
it's children) don't have side-effects AND the nodes after it don't
use the values produced by the node.

The only difference is when evaluating side effects: loops consider
only return jumps as a side-effect -- they can stop execution of nodes
after it; 'if' nodes outside loops should consider all kinds of
jumps (return, break, continue) since all of them can cause execution
of nodes after it to be skipped.

After this patch, empty ifs (those which both then and else blocks are
empty) will be removed by nir_opt_dead_cf.

It caused no change to shader-db, in part because the removal of empty
ifs is currently covered by nir_opt_peephole_select.

v2: Improve the identification of cases where break/continue can cause
    side-effects. (Jason)

v3: Move code comment changes to a different patch. (Jason)
---
 src/compiler/nir/nir_opt_dead_cf.c | 44 ++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/src/compiler/nir/nir_opt_dead_cf.c b/src/compiler/nir/nir_opt_dead_cf.c
index b15dfdd0c9..a652bcd99b 100644
--- a/src/compiler/nir/nir_opt_dead_cf.c
+++ b/src/compiler/nir/nir_opt_dead_cf.c
@@ -60,12 +60,12 @@
  * }
  * ...
  *
- * Finally, we also handle removing useless loops, i.e. loops with no side
- * effects and without any definitions that are used elsewhere. This case is a
- * little different from the first two in that the code is actually run (it
- * just never does anything), but there are similar issues with needing to
- * be careful with restarting after deleting the cf_node (see dead_cf_list())
- * so this is a convenient place to remove them.
+ * Finally, we also handle removing useless loops and ifs, i.e. loops and ifs
+ * with no side effects and without any definitions that are used
+ * elsewhere. This case is a little different from the first two in that the
+ * code is actually run (it just never does anything), but there are similar
+ * issues with needing to be careful with restarting after deleting the
+ * cf_node (see dead_cf_list()) so this is a convenient place to remove them.
  */
 
 static void
@@ -136,6 +136,12 @@ static bool
 cf_node_has_side_effects(nir_cf_node *node)
 {
    nir_foreach_block_in_cf_node(block, node) {
+      bool inside_loop = node->type == nir_cf_node_loop;
+      for (nir_cf_node *n = &block->cf_node; !inside_loop && n != node; n = n->parent) {
+         if (n->type == nir_cf_node_loop)
+            inside_loop = true;
+      }
+
       nir_foreach_instr(instr, block) {
          if (instr->type == nir_instr_type_call)
             return true;
@@ -143,10 +149,13 @@ cf_node_has_side_effects(nir_cf_node *node)
          /* Return instructions can cause us to skip over other side-effecting
           * instructions after the loop, so consider them to have side effects
           * here.
+          *
+          * When the block is not inside a loop, break and continue might also
+          * cause a skip.
           */
 
          if (instr->type == nir_instr_type_jump &&
-             nir_instr_as_jump(instr)->type == nir_jump_return)
+             (!inside_loop || nir_instr_as_jump(instr)->type == nir_jump_return))
             return true;
 
          if (instr->type != nir_instr_type_intrinsic)
@@ -171,7 +180,7 @@ def_not_live_out(nir_ssa_def *def, void *state)
 }
 
 /*
- * Test if a loop is dead. A loop node is dead if:
+ * Test if a loop node or if node is dead. Such nodes are dead if:
  *
  * 1) It has no side effects (i.e. intrinsics which could possibly affect the
  * state of the program aside from producing an SSA value, indicated by a lack
@@ -187,19 +196,21 @@ def_not_live_out(nir_ssa_def *def, void *state)
  */
 
 static bool
-loop_is_dead(nir_loop *loop)
+node_is_dead(nir_cf_node *node)
 {
-   nir_block *before = nir_cf_node_as_block(nir_cf_node_prev(&loop->cf_node));
-   nir_block *after = nir_cf_node_as_block(nir_cf_node_next(&loop->cf_node));
+   assert(node->type == nir_cf_node_loop || node->type == nir_cf_node_if);
+
+   nir_block *before = nir_cf_node_as_block(nir_cf_node_prev(node));
+   nir_block *after = nir_cf_node_as_block(nir_cf_node_next(node));
 
    if (!exec_list_is_empty(&after->instr_list) &&
        nir_block_first_instr(after)->type == nir_instr_type_phi)
       return false;
 
-   if (cf_node_has_side_effects(&loop->cf_node))
+   if (cf_node_has_side_effects(node))
       return false;
 
-   nir_function_impl *impl = nir_cf_node_get_function(&loop->cf_node);
+   nir_function_impl *impl = nir_cf_node_get_function(node);
    nir_metadata_require(impl, nir_metadata_live_ssa_defs |
                               nir_metadata_dominance);
 
@@ -219,6 +230,11 @@ dead_cf_block(nir_block *block)
 {
    nir_if *following_if = nir_block_get_following_if(block);
    if (following_if) {
+      if (node_is_dead(&following_if->cf_node)) {
+         nir_cf_node_remove(&following_if->cf_node);
+         return true;
+      }
+
       nir_const_value *const_value =
          nir_src_as_const_value(following_if->condition);
 
@@ -233,7 +249,7 @@ dead_cf_block(nir_block *block)
    if (!following_loop)
       return false;
 
-   if (!loop_is_dead(following_loop))
+   if (!node_is_dead(&following_loop->cf_node))
       return false;
 
    nir_cf_node_remove(&following_loop->cf_node);
-- 
2.16.2