nir: Clean up nir_op_is_vec() and its callers

The nir_op_is_vec() helper I added in 842338e2f0 ("nir: Add a
nir_op_is_vec helper") treats nir_op_mov as a vec even though the
semanitcs of the two are different.  In retrospect, this was a mistake
as the previous three fixup commits show.  This commit splits the helper
into two: nir_op_is_vec() and nir_op_is_vec_or_mov() and uses the
appropriate helper at each call site.  Hopefully, this rename will
encurage any future users of these helpers to think about nir_op_mov as
separate from nir_op_vecN and we can avoid these bugs.

Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24704>

src/amd/common/ac_nir_lower_ngg.c

@@ -793,7 +793,7 @@ remove_extra_pos_output(nir_builder *b, nir_instr *instr, void *state)
 
    if (store_val->parent_instr->type == nir_instr_type_alu) {
       nir_alu_instr *alu = nir_instr_as_alu(store_val->parent_instr);
-      if (nir_op_is_vec(alu->op)) {
+      if (nir_op_is_vec_or_mov(alu->op)) {
          /* Output store uses a vector, we can easily rewrite uses of each vector element. */
 
          unsigned num_vec_src = 0;

src/asahi/compiler/agx_compile.c

@@ -1229,9 +1229,10 @@ agx_emit_alu(agx_builder *b, nir_alu_instr *instr)
    unsigned src_sz = srcs ? nir_src_bit_size(instr->src[0].src) : 0;
    ASSERTED unsigned comps = instr->def.num_components;
 
-   assert(comps == 1 || nir_op_is_vec(instr->op));
+   assert(comps == 1 || nir_op_is_vec_or_mov(instr->op));
-   assert(sz == 1 ||
+   assert(
-          ((nir_op_is_vec(instr->op) || is_conversion_to_8bit(instr->op)) &&
+      sz == 1 ||
+      ((nir_op_is_vec_or_mov(instr->op) || is_conversion_to_8bit(instr->op)) &&
        sz == 8) ||
       sz == 16 || sz == 32 || sz == 64);
 

src/asahi/compiler/agx_nir_opt_preamble.c

@@ -39,7 +39,7 @@ instr_cost(nir_instr *instr, const void *data)
 
    case nir_instr_type_alu:
       /* We optimistically assume that moves get coalesced */
-      if (nir_op_is_vec(nir_instr_as_alu(instr)->op))
+      if (nir_op_is_vec_or_mov(nir_instr_as_alu(instr)->op))
          return 0.0;
       else
          return 2.0;

src/compiler/nir/nir.c

@@ -2597,9 +2597,9 @@ nir_chase_binding(nir_src rsrc)
    }
 
    /* Skip copies and trimming. Trimming can appear as nir_op_mov instructions
-    * when removing the offset from addresses. We also consider nir_op_is_vec()
+    * when removing the offset from addresses. We also consider
-    * instructions to skip trimming of vec2_index_32bit_offset addresses after
+    * nir_op_is_vec_or_mov() instructions to skip trimming of
-    * lowering ALU to scalar.
+    * vec2_index_32bit_offset addresses after lowering ALU to scalar.
     */
    unsigned num_components = nir_src_num_components(rsrc);
    while (true) {
@@ -2708,12 +2708,6 @@ nir_get_binding_variable(nir_shader *shader, nir_binding binding)
    return binding_var;
 }
 
-bool
-nir_alu_instr_is_copy(nir_alu_instr *instr)
-{
-   return nir_op_is_vec(instr->op);
-}
-
 nir_scalar
 nir_scalar_chase_movs(nir_scalar s)
 {
@@ -2792,10 +2786,10 @@ nir_get_glsl_base_type_for_nir_type(nir_alu_type base_type)
 }
 
 nir_op
-nir_op_vec(unsigned components)
+nir_op_vec(unsigned num_components)
 {
    /* clang-format off */
-   switch (components) {
+   switch (num_components) {
    case  1: return nir_op_mov;
    case  2: return nir_op_vec2;
    case  3: return nir_op_vec3;
@@ -2812,7 +2806,6 @@ bool
 nir_op_is_vec(nir_op op)
 {
    switch (op) {
-   case nir_op_mov:
    case nir_op_vec2:
    case nir_op_vec3:
    case nir_op_vec4:

src/compiler/nir/nir.h

@@ -1231,12 +1231,35 @@ nir_atomic_op_type(nir_atomic_op op)
    unreachable("Invalid nir_atomic_op");
 }
 
+/** Returns nir_op_vec<num_components> or nir_op_mov if num_components == 1
+ *
+ * This is subtly different from nir_op_is_vec() which returns false for
+ * nir_op_mov.  Returning nir_op_mov from nir_op_vec() when num_components == 1
+ * makes sense under the assumption that the num_components of the resulting
+ * nir_def will same as what is passed in here because a single-component mov
+ * is effectively a vec1.  However, if alu->def.num_components > 1, nir_op_mov
+ * has different semantics from nir_op_vec* so so code which detects "is this
+ * a vec?" typically needs to handle nir_op_mov separate from nir_op_vecN.
+ *
+ * In the unlikely case where you can handle nir_op_vecN and nir_op_mov
+ * together, use nir_op_is_vec_or_mov().
+ */
 nir_op
-nir_op_vec(unsigned components);
+nir_op_vec(unsigned num_components);
 
+/** Returns true if this op is one of nir_op_vec*
+ *
+ * Returns false for nir_op_mov.  See nir_op_vec() for more details.
+ */
 bool
 nir_op_is_vec(nir_op op);
 
+static inline bool
+nir_op_is_vec_or_mov(nir_op op)
+{
+   return op == nir_op_mov || nir_op_is_vec(op);
+}
+
 static inline bool
 nir_is_float_control_signed_zero_inf_nan_preserve(unsigned execution_mode, unsigned bit_size)
 {
@@ -1449,8 +1472,6 @@ typedef struct nir_alu_instr {
 void nir_alu_src_copy(nir_alu_src *dest, const nir_alu_src *src,
                       nir_alu_instr *instr);
 
-bool nir_alu_instr_is_copy(nir_alu_instr *instr);
-
 /* is this source channel used? */
 bool
 nir_alu_instr_channel_used(const nir_alu_instr *instr, unsigned src,

src/compiler/nir/nir_lower_phis_to_scalar.c

@@ -61,7 +61,7 @@ is_phi_src_scalarizable(nir_phi_src *src,
        * are ok too.
        */
       return nir_op_infos[src_alu->op].output_size == 0 ||
-             nir_op_is_vec(src_alu->op);
+             nir_op_is_vec_or_mov(src_alu->op);
    }
 
    case nir_instr_type_phi:

src/compiler/nir/nir_lower_vec_to_regs.c

@@ -195,7 +195,7 @@ lower(nir_builder *b, nir_instr *instr, void *data_)
       return false;
 
    nir_alu_instr *vec = nir_instr_as_alu(instr);
-   if (vec->op == nir_op_mov || !nir_op_is_vec(vec->op))
+   if (!nir_op_is_vec(vec->op))
       return false;
 
    unsigned num_components = vec->def.num_components;

src/compiler/nir/nir_opt_copy_propagate.c

@@ -128,7 +128,7 @@ copy_prop_instr(nir_instr *instr)
 
    nir_alu_instr *mov = nir_instr_as_alu(instr);
 
-   if (!nir_alu_instr_is_copy(mov))
+   if (!nir_op_is_vec_or_mov(mov->op))
       return false;
 
    bool progress = false;

src/compiler/nir/nir_opt_if.c

@@ -399,7 +399,7 @@ opt_split_alu_of_phi(nir_builder *b, nir_loop *loop)
        * to loop unrolling not recognizing loop termintators, and type
        * conversions also lead to regressions.
        */
-      if (nir_op_is_vec(alu->op) ||
+      if (nir_op_is_vec_or_mov(alu->op) ||
           nir_alu_instr_is_comparison(alu) ||
           alu_instr_is_type_conversion(alu))
          continue;

src/compiler/nir/nir_opt_shrink_vectors.c

@@ -439,7 +439,7 @@ opt_shrink_vectors_phi(nir_builder *b, nir_phi_instr *instr)
       /* However, even if the instruction only points back at the phi, we still
        * need to check that the swizzles are trivial.
        */
-      if (nir_op_is_vec(alu->op) && alu->op != nir_op_mov) {
+      if (nir_op_is_vec(alu->op)) {
          if (src_idx != alu->src[src_idx].swizzle[0]) {
             mask |= src_read_mask;
          }

src/compiler/nir/nir_opt_sink.c

@@ -44,7 +44,7 @@ nir_can_move_instr(nir_instr *instr, nir_move_options options)
       return options & nir_move_const_undef;
    }
    case nir_instr_type_alu: {
-      if (nir_op_is_vec(nir_instr_as_alu(instr)->op) ||
+      if (nir_op_is_vec_or_mov(nir_instr_as_alu(instr)->op) ||
           nir_instr_as_alu(instr)->op == nir_op_b2i32)
          return options & nir_move_copies;
       if (nir_alu_instr_is_comparison(nir_instr_as_alu(instr)))

src/compiler/nir/nir_opt_undef.c

@@ -72,7 +72,7 @@ opt_undef_csel(nir_alu_instr *instr)
 static bool
 opt_undef_vecN(nir_builder *b, nir_alu_instr *alu)
 {
-   if (!nir_op_is_vec(alu->op))
+   if (!nir_op_is_vec_or_mov(alu->op))
       return false;
 
    for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {
@@ -103,7 +103,7 @@ nir_get_undef_mask(nir_def *def)
    unsigned undef = 0;
 
    /* nir_op_mov of undef is handled by opt_undef_vecN() */
-   if (nir_op_is_vec(alu->op) && alu->op != nir_op_mov) {
+   if (nir_op_is_vec(alu->op)) {
       for (int i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {
          if (alu->src[i].src.ssa->parent_instr->type ==
              nir_instr_type_undef) {

src/compiler/nir/tests/vars_tests.cpp

@@ -1948,7 +1948,7 @@ vec_src_comp_as_int(nir_src src, unsigned comp)
       return nir_src_comp_as_int(src, comp);
 
    nir_scalar s = { src.ssa, comp };
-   assert(nir_op_is_vec(nir_scalar_alu_op(s)));
+   assert(nir_op_is_vec_or_mov(nir_scalar_alu_op(s)));
    return nir_scalar_as_int(nir_scalar_chase_alu_src(s, comp));
 }
 

src/gallium/auxiliary/nir/nir_to_tgsi.c

@@ -570,9 +570,6 @@ ntt_extract_const_src_offset(nir_src *src)
       /* We'd like to reuse nir_scalar_chase_movs(), but it assumes SSA and that
        * seems reasonable for something used in inner loops of the compiler.
        */
-      if (!nir_alu_instr_is_copy(alu))
-         return 0;
-
       if (alu->op == nir_op_mov) {
          s.def = alu->src[0].src.ssa;
          s.comp = alu->src[0].swizzle[s.comp];

src/microsoft/compiler/nir_to_dxil.c

@@ -6119,7 +6119,7 @@ lower_bit_size_callback(const nir_instr* instr, void *data)
    if (nir_op_infos[alu->op].is_conversion)
       return 0;
 
-   if (nir_alu_instr_is_copy(alu))
+   if (nir_op_is_vec_or_mov(alu->op))
       return 0;
 
    unsigned num_inputs = nir_op_infos[alu->op].num_inputs;