nir: evaluate if condition uses inside the if branches
authorTimothy Arceri <tarceri@itsqueeze.com>
Wed, 29 Aug 2018 15:24:43 +0000 (10:24 -0500)
committerTimothy Arceri <tarceri@itsqueeze.com>
Fri, 14 Sep 2018 06:07:36 +0000 (16:07 +1000)
Since we know what side of the branch we ended up on we can just
replace the use with a constant.

All the spill changes in shader-db are from Dolphin uber shaders,
despite some small regressions the change is clearly positive.

V2: insert new constant after any phis in the
    use->parent_instr->type == nir_instr_type_phi path.

v3:
 - use nir_after_block_before_jump() for inserting const
 - check dominance of phi uses correctly

v4:
 - create some helpers as suggested by Jason.

v5 (Jason Ekstrand):
 - Use LIST_ENTRY to get the phi src

shader-db results IVB:

total instructions in shared programs: 9999201 -> 9993483 (-0.06%)
instructions in affected programs: 163235 -> 157517 (-3.50%)
helped: 132
HURT: 2

total cycles in shared programs: 231670754 -> 219476091 (-5.26%)
cycles in affected programs: 143424120 -> 131229457 (-8.50%)
helped: 115
HURT: 24

total spills in shared programs: 4383 -> 4370 (-0.30%)
spills in affected programs: 1656 -> 1643 (-0.79%)
helped: 9
HURT: 18

total fills in shared programs: 4610 -> 4581 (-0.63%)
fills in affected programs: 374 -> 345 (-7.75%)
helped: 6
HURT: 0

Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
src/compiler/nir/nir.h
src/compiler/nir/nir_opt_if.c

index bf4bd916d27c99eb929a1c8fb15baaa652007a03..599f469a714f255d82b1a5f4b2525848ee440bcf 100644 (file)
@@ -2353,6 +2353,36 @@ nir_after_block_before_jump(nir_block *block)
    }
 }
 
+static inline nir_cursor
+nir_before_src(nir_src *src, bool is_if_condition)
+{
+   if (is_if_condition) {
+      nir_block *prev_block =
+         nir_cf_node_as_block(nir_cf_node_prev(&src->parent_if->cf_node));
+      assert(!nir_block_ends_in_jump(prev_block));
+      return nir_after_block(prev_block);
+   } else if (src->parent_instr->type == nir_instr_type_phi) {
+#ifndef NDEBUG
+      nir_phi_instr *cond_phi = nir_instr_as_phi(src->parent_instr);
+      bool found = false;
+      nir_foreach_phi_src(phi_src, cond_phi) {
+         if (phi_src->src.ssa == src->ssa) {
+            found = true;
+            break;
+         }
+      }
+      assert(found);
+#endif
+      /* The LIST_ENTRY macro is a generic container-of macro, it just happens
+       * to have a more specific name.
+       */
+      nir_phi_src *phi_src = LIST_ENTRY(nir_phi_src, src, src);
+      return nir_after_block_before_jump(phi_src->pred);
+   } else {
+      return nir_before_instr(src->parent_instr);
+   }
+}
+
 static inline nir_cursor
 nir_before_cf_node(nir_cf_node *node)
 {
index dacf2d6c667a5a2733747815aecb3196dd36d37d..512fd92575ba5eaffee4f60eec1b93978a3c6491 100644 (file)
@@ -369,6 +369,73 @@ opt_if_loop_terminator(nir_if *nif)
    return true;
 }
 
+static void
+replace_if_condition_use_with_const(nir_builder *b, nir_src *use,
+                                    nir_const_value nir_boolean,
+                                    bool if_condition)
+{
+   /* Create const */
+   nir_ssa_def *const_def = nir_build_imm(b, 1, 32, nir_boolean);
+
+   /* Rewrite use to use const */
+   nir_src new_src = nir_src_for_ssa(const_def);
+   if (if_condition)
+      nir_if_rewrite_condition(use->parent_if, new_src);
+   else
+      nir_instr_rewrite_src(use->parent_instr, use, new_src);
+}
+
+static bool
+evaluate_if_condition(nir_if *nif, nir_cursor cursor, uint32_t *value)
+{
+   nir_block *use_block = nir_cursor_current_block(cursor);
+   if (nir_block_dominates(nir_if_first_then_block(nif), use_block)) {
+      *value = NIR_TRUE;
+      return true;
+   } else if (nir_block_dominates(nir_if_first_else_block(nif), use_block)) {
+      *value = NIR_FALSE;
+      return true;
+   } else {
+      return false;
+   }
+}
+
+static bool
+evaluate_condition_use(nir_builder *b, nir_if *nif, nir_src *use_src,
+                       bool is_if_condition)
+{
+   bool progress = false;
+
+   nir_const_value value;
+   b->cursor = nir_before_src(use_src, is_if_condition);
+
+   if (evaluate_if_condition(nif, b->cursor, &value.u32[0])) {
+      replace_if_condition_use_with_const(b, use_src, value, is_if_condition);
+      progress = true;
+   }
+
+   return progress;
+}
+
+static bool
+opt_if_evaluate_condition_use(nir_builder *b, nir_if *nif)
+{
+   bool progress = false;
+
+   /* Evaluate any uses of the if condition inside the if branches */
+   assert(nif->condition.is_ssa);
+   nir_foreach_use_safe(use_src, nif->condition.ssa) {
+      progress |= evaluate_condition_use(b, nif, use_src, false);
+   }
+
+   nir_foreach_if_use_safe(use_src, nif->condition.ssa) {
+      if (use_src->parent_if != nif)
+         progress |= evaluate_condition_use(b, nif, use_src, true);
+   }
+
+   return progress;
+}
+
 static bool
 opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
 {
@@ -402,6 +469,41 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
    return progress;
 }
 
+/**
+ * These optimisations depend on nir_metadata_block_index and therefore must
+ * not do anything to cause the metadata to become invalid.
+ */
+static bool
+opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list)
+{
+   bool progress = false;
+   foreach_list_typed(nir_cf_node, cf_node, node, cf_list) {
+      switch (cf_node->type) {
+      case nir_cf_node_block:
+         break;
+
+      case nir_cf_node_if: {
+         nir_if *nif = nir_cf_node_as_if(cf_node);
+         progress |= opt_if_safe_cf_list(b, &nif->then_list);
+         progress |= opt_if_safe_cf_list(b, &nif->else_list);
+         progress |= opt_if_evaluate_condition_use(b, nif);
+         break;
+      }
+
+      case nir_cf_node_loop: {
+         nir_loop *loop = nir_cf_node_as_loop(cf_node);
+         progress |= opt_if_safe_cf_list(b, &loop->body);
+         break;
+      }
+
+      case nir_cf_node_function:
+         unreachable("Invalid cf type");
+      }
+   }
+
+   return progress;
+}
+
 bool
 nir_opt_if(nir_shader *shader)
 {
@@ -414,6 +516,12 @@ nir_opt_if(nir_shader *shader)
       nir_builder b;
       nir_builder_init(&b, function->impl);
 
+      nir_metadata_require(function->impl, nir_metadata_block_index |
+                           nir_metadata_dominance);
+      progress = opt_if_safe_cf_list(&b, &function->impl->body);
+      nir_metadata_preserve(function->impl, nir_metadata_block_index |
+                            nir_metadata_dominance);
+
       if (opt_if_cf_list(&b, &function->impl->body)) {
          nir_metadata_preserve(function->impl, nir_metadata_none);