nir/copy_prop: visit copies instead of sources

There are less copy instructions than sources, so instead of visiting each
source and rewriting it if it's uses a copy instruction, visit each copy
instruction and rewrite it's users.

Besides improving compile time, this also has a side effect of fixing a
rare situation where copy-propagation does not happen:
loop {
   a = phi ..., b
   c = vec ...
   b = mov c.y
}
It might have been the case that a phi source could not be rewritten until
the copy was visited later.

Compile-time (nir_copy_prop):
Difference at 95.0% confidence
	-2613.13 +/- 15.2094
	-27.4333% +/- 0.150247%
	(Student's t, pooled s = 17.963)

Comple-time (overall):
Difference at 95.0% confidence
	-2627.89 +/- 201.557
	-2.05404% +/- 0.156221%
	(Student's t, pooled s = 238.048)

Signed-off-by: Rhys Perry <pendingchaos02@gmail.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8784>

diff --git a/src/compiler/nir/nir_opt_copy_propagate.c b/src/compiler/nir/nir_opt_copy_propagate.c
index 52c8ace..bba4ed9 100644 (file)
--- a/src/compiler/nir/nir_opt_copy_propagate.c
+++ b/src/compiler/nir/nir_opt_copy_propagate.c
@@ -26,249 +26,162 @@
  */
 
 #include "nir.h"
+#include "nir_builder.h"
 
 /**
  * SSA-based copy propagation
  */
 
-static bool is_move(nir_alu_instr *instr)
+static bool
+is_swizzleless_move(nir_alu_instr *instr)
 {
-   assert(instr->src[0].src.is_ssa);
+   unsigned num_comp = instr->dest.dest.ssa.num_components;
 
-   if (instr->op != nir_op_mov)
+   if (instr->src[0].src.ssa->num_components != num_comp)
       return false;
 
-   if (instr->dest.saturate)
-      return false;
-
-   /* we handle modifiers in a separate pass */
-
-   if (instr->src[0].abs || instr->src[0].negate)
-      return false;
-
-   return true;
-
-}
-
-static bool is_vec(nir_alu_instr *instr)
-{
-   for (unsigned i = 0; i < nir_op_infos[instr->op].num_inputs; i++) {
-      assert(instr->src[i].src.is_ssa);
-
-      /* we handle modifiers in a separate pass */
-      if (instr->src[i].abs || instr->src[i].negate)
-         return false;
+   if (instr->op == nir_op_mov) {
+      for (unsigned i = 0; i < num_comp; i++) {
+         if (instr->src[0].swizzle[i] != i)
+            return false;
+      }
+   } else {
+      for (unsigned i = 0; i < num_comp; i++) {
+         if (instr->src[i].swizzle[0] != i ||
+             instr->src[i].src.ssa != instr->src[0].src.ssa)
+            return false;
+      }
    }
 
-   assert(instr->dest.dest.is_ssa);
-   return nir_op_is_vec(instr->op);
+   return true;
 }
 
 static bool
-is_swizzleless_move(nir_alu_instr *instr)
+is_copy(nir_alu_instr *instr)
 {
-   if (is_move(instr)) {
-      for (unsigned i = 0; i < NIR_MAX_VEC_COMPONENTS; i++) {
-         if (!((instr->dest.write_mask >> i) & 1))
-            break;
-         if (instr->src[0].swizzle[i] != i)
-            return false;
-      }
-      return true;
-   } else if (is_vec(instr)) {
-      nir_ssa_def *def = NULL;
-      for (unsigned i = 0; i < nir_op_infos[instr->op].num_inputs; i++) {
-         if (instr->src[i].swizzle[0] != i)
-            return false;
+   assert(instr->src[0].src.is_ssa);
 
-         if (def == NULL) {
-            def = instr->src[i].src.ssa;
-         } else if (instr->src[i].src.ssa != def) {
+   /* we handle modifiers in a separate pass */
+   if (instr->op == nir_op_mov) {
+      return !instr->dest.saturate &&
+             !instr->src[0].abs &&
+             !instr->src[0].negate;
+   } else if (nir_op_is_vec(instr->op)) {
+      for (unsigned i = 0; i < instr->dest.dest.ssa.num_components; i++) {
+         if (instr->src[i].abs || instr->src[i].negate)
             return false;
-         }
       }
-      return true;
+      return !instr->dest.saturate;
    } else {
       return false;
    }
 }
 
 static bool
-copy_prop_src(nir_src *src, nir_instr *parent_instr, nir_if *parent_if,
-              unsigned num_components)
+rewrite_to_vec(nir_function_impl *impl, nir_alu_instr *mov, nir_alu_instr *vec)
 {
-   assert(src->is_ssa);
+   if (mov->op != nir_op_mov)
+      return false;
 
-   nir_ssa_def *original_ssa = src->ssa;
-   nir_instr *src_instr = original_ssa->parent_instr;
-   nir_ssa_def *copy_def;
-   if (src_instr->type == nir_instr_type_alu) {
-      nir_alu_instr *alu_instr = nir_instr_as_alu(src_instr);
-      if (!is_swizzleless_move(alu_instr))
-         return false;
+   nir_builder b;
+   nir_builder_init(&b, impl);
+   b.cursor = nir_after_instr(&mov->instr);
 
-      if (alu_instr->src[0].src.ssa->num_components != num_components)
-         return false;
+   unsigned num_comp = mov->dest.dest.ssa.num_components;
+   nir_alu_instr *new_vec = nir_alu_instr_create(b.shader, nir_op_vec(num_comp));
+   for (unsigned i = 0; i < num_comp; i++)
+      new_vec->src[i] = vec->src[mov->src[0].swizzle[i]];
 
-      copy_def= alu_instr->src[0].src.ssa;
-   } else {
-      return false;
-   }
+   nir_ssa_def *new = nir_builder_alu_instr_finish_and_insert(&b, new_vec);
+   nir_ssa_def_rewrite_uses_ssa(&mov->dest.dest.ssa, new);
 
-   if (parent_instr) {
-      nir_instr_rewrite_src(parent_instr, src, nir_src_for_ssa(copy_def));
-   } else {
-      assert(src == &parent_if->condition);
-      nir_if_rewrite_condition(parent_if, nir_src_for_ssa(copy_def));
-   }
-
-   if (nir_ssa_def_is_unused(original_ssa))
-      nir_instr_remove(src_instr);
+   /* If we remove "mov" and it's the next instruction in the
+    * nir_foreach_instr_safe() loop, then we would end copy-propagation early. */
 
    return true;
 }
 
 static bool
-copy_prop_alu_src(nir_alu_instr *parent_alu_instr, unsigned index)
+copy_propagate_alu(nir_function_impl *impl, nir_alu_src *src, nir_alu_instr *copy)
 {
-   nir_alu_src *src = &parent_alu_instr->src[index];
-   assert(src->src.is_ssa);
-
-   nir_ssa_def *original_ssa = src->src.ssa;
-   nir_instr *src_instr = original_ssa->parent_instr;
-   if (src_instr->type != nir_instr_type_alu)
-      return false;
-
-   nir_alu_instr *alu_instr = nir_instr_as_alu(src_instr);
-   if (!is_move(alu_instr) && !is_vec(alu_instr))
-      return false;
+   nir_ssa_def *def = NULL;
+   nir_alu_instr *user = nir_instr_as_alu(src->src.parent_instr);
+   unsigned src_idx = src - user->src;
+   assert(src_idx < nir_op_infos[user->op].num_inputs);
+   unsigned num_comp = nir_ssa_alu_instr_src_components(user, src_idx);
 
-   nir_ssa_def *def;
-   unsigned new_swizzle[NIR_MAX_VEC_COMPONENTS] = {0, 0, 0, 0};
+   if (copy->op == nir_op_mov) {
+      def = copy->src[0].src.ssa;
 
-   if (alu_instr->op == nir_op_mov) {
-      for (unsigned i = 0; i < NIR_MAX_VEC_COMPONENTS; i++)
-         new_swizzle[i] = alu_instr->src[0].swizzle[src->swizzle[i]];
-      def = alu_instr->src[0].src.ssa;
+      for (unsigned i = 0; i < num_comp; i++)
+         src->swizzle[i] = copy->src[0].swizzle[src->swizzle[i]];
    } else {
-      def = NULL;
-
-      for (unsigned i = 0; i < NIR_MAX_VEC_COMPONENTS; i++) {
-         if (!nir_alu_instr_channel_used(parent_alu_instr, index, i))
-            continue;
-
-         nir_ssa_def *new_def = alu_instr->src[src->swizzle[i]].src.ssa;
-         if (def == NULL)
-            def = new_def;
-         else {
-            if (def != new_def)
-               return false;
-         }
-         new_swizzle[i] = alu_instr->src[src->swizzle[i]].swizzle[0];
-      }
-   }
+      def = copy->src[src->swizzle[0]].src.ssa;
 
-   for (unsigned i = 0; i < NIR_MAX_VEC_COMPONENTS; i++)
-      src->swizzle[i] = new_swizzle[i];
+      for (unsigned i = 1; i < num_comp; i++) {
+         if (copy->src[src->swizzle[i]].src.ssa != def)
+            return rewrite_to_vec(impl, user, copy);
+      }
 
-   nir_instr_rewrite_src(&parent_alu_instr->instr, &src->src,
-                         nir_src_for_ssa(def));
+      for (unsigned i = 0; i < num_comp; i++)
+         src->swizzle[i] = copy->src[src->swizzle[i]].swizzle[0];
+   }
 
-   if (nir_ssa_def_is_unused(original_ssa))
-      nir_instr_remove(src_instr);
+   nir_instr_rewrite_src(&user->instr, &src->src, nir_src_for_ssa(def));
 
    return true;
 }
 
 static bool
-copy_prop_instr(nir_instr *instr)
+copy_propagate(nir_src *src, nir_alu_instr *copy)
 {
-   bool progress = false;
-   switch (instr->type) {
-   case nir_instr_type_alu: {
-      nir_alu_instr *alu_instr = nir_instr_as_alu(instr);
-
-      for (unsigned i = 0; i < nir_op_infos[alu_instr->op].num_inputs; i++)
-         while (copy_prop_alu_src(alu_instr, i))
-            progress = true;
-
-      return progress;
-   }
-
-   case nir_instr_type_deref: {
-      nir_deref_instr *deref = nir_instr_as_deref(instr);
+   if (!is_swizzleless_move(copy))
+      return false;
 
-      if (deref->deref_type != nir_deref_type_var) {
-         assert(deref->dest.is_ssa);
-         const unsigned comps = deref->dest.ssa.num_components;
-         while (copy_prop_src(&deref->parent, instr, NULL, comps))
-            progress = true;
-      }
+   nir_instr_rewrite_src(src->parent_instr, src, nir_src_for_ssa(copy->src[0].src.ssa));
 
-      if (deref->deref_type == nir_deref_type_array ||
-          deref->deref_type == nir_deref_type_ptr_as_array) {
-         while (copy_prop_src(&deref->arr.index, instr, NULL, 1))
-            progress = true;
-      }
+   return true;
+}
 
-      return progress;
-   }
+static bool
+copy_propagate_if(nir_src *src, nir_alu_instr *copy)
+{
+   if (!is_swizzleless_move(copy))
+      return false;
 
-   case nir_instr_type_tex: {
-      nir_tex_instr *tex = nir_instr_as_tex(instr);
-      for (unsigned i = 0; i < tex->num_srcs; i++) {
-         unsigned num_components = nir_tex_instr_src_size(tex, i);
-         while (copy_prop_src(&tex->src[i].src, instr, NULL, num_components))
-            progress = true;
-      }
+   nir_if *parent_if = container_of(src, nir_if, condition);
+   nir_if_rewrite_condition(parent_if, nir_src_for_ssa(copy->src[0].src.ssa));
 
-      return progress;
-   }
+   return true;
+}
 
-   case nir_instr_type_intrinsic: {
-      nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr);
-      for (unsigned i = 0;
-           i < nir_intrinsic_infos[intrin->intrinsic].num_srcs; i++) {
-         unsigned num_components = nir_intrinsic_src_components(intrin, i);
+static bool
+copy_prop_instr(nir_function_impl *impl, nir_instr *instr)
+{
+   if (instr->type != nir_instr_type_alu)
+      return false;
 
-         while (copy_prop_src(&intrin->src[i], instr, NULL, num_components))
-            progress = true;
-      }
+   nir_alu_instr *mov = nir_instr_as_alu(instr);
 
-      return progress;
-   }
+   if (!is_copy(mov))
+      return false;
 
-   case nir_instr_type_jump: {
-      nir_jump_instr *jump = nir_instr_as_jump(instr);
-      if (jump->type == nir_jump_goto_if) {
-         while (copy_prop_src(&jump->condition, instr, NULL, 1))
-            progress = true;
-      }
+   bool progress = false;
 
-      return progress;
+   nir_foreach_use_safe(src, &mov->dest.dest.ssa) {
+      if (src->parent_instr->type == nir_instr_type_alu)
+         progress |= copy_propagate_alu(impl, container_of(src, nir_alu_src, src), mov);
+      else
+         progress |= copy_propagate(src, mov);
    }
 
-   case nir_instr_type_phi: {
-      nir_phi_instr *phi = nir_instr_as_phi(instr);
-      assert(phi->dest.is_ssa);
-      unsigned num_components = phi->dest.ssa.num_components;
-      nir_foreach_phi_src(src, phi) {
-         while (copy_prop_src(&src->src, instr, NULL, num_components))
-            progress = true;
-      }
-
-      return progress;
-   }
+   nir_foreach_if_use_safe(src, &mov->dest.dest.ssa)
+      progress |= copy_propagate_if(src, mov);
 
-   default:
-      return false;
-   }
-}
+   if (progress && nir_ssa_def_is_unused(&mov->dest.dest.ssa))
+      nir_instr_remove(&mov->instr);
 
-static bool
-copy_prop_if(nir_if *if_stmt)
-{
-   return copy_prop_src(&if_stmt->condition, NULL, if_stmt, 1);
+   return progress;
 }
 
 static bool
@@ -277,15 +190,10 @@ nir_copy_prop_impl(nir_function_impl *impl)
    bool progress = false;
 
    nir_foreach_block(block, impl) {
-      nir_foreach_instr(instr, block) {
-         if (copy_prop_instr(instr))
-            progress = true;
-      }
-
-      nir_if *if_stmt = nir_block_get_following_if(block);
-      if (if_stmt && copy_prop_if(if_stmt))
-         progress = true;
+      nir_foreach_instr_safe(instr, block) {
+         progress |= copy_prop_instr(impl, instr);
       }
+   }
 
    if (progress) {
       nir_metadata_preserve(impl, nir_metadata_block_index |