nir/validate: Validate that bit sizes and components always match

We've always required bit sizes to match but the rules for number of
components have been a bit loose.  You've never been allowed to source
from something with less components than you consume, but more has
always been fine.  This changes the validator to require that they match
exactly.  The fact that they don't always match has been a source of
confusion in NIR for quite some time and it's time we got rid of it.

Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Connor Abbott <cwabbott0@gmail.com>

diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_validate.c
index 15ba65f2b1205450c0d391b18de6b46a78fcb5ff..16efcb2356c0832829f6d0e8e5413f2f8dfd4ef2 100644 (file)
--- a/src/compiler/nir/nir_validate.c
+++ b/src/compiler/nir/nir_validate.c
@@ -126,10 +126,12 @@ log_error(validate_state *state, const char *cond, const char *file, int line)
          log_error(state, #cond, __FILE__, __LINE__); \
    } while (0)
 
-static void validate_src(nir_src *src, validate_state *state);
+static void validate_src(nir_src *src, validate_state *state,
+                         unsigned bit_size, unsigned num_components);
 
 static void
-validate_reg_src(nir_src *src, validate_state *state)
+validate_reg_src(nir_src *src, validate_state *state,
+                 unsigned bit_size, unsigned num_components)
 {
    validate_assert(state, src->reg.reg != NULL);
 
@@ -151,6 +153,13 @@ validate_reg_src(nir_src *src, validate_state *state)
              "using a register declared in a different function");
    }
 
+   if (!src->reg.reg->is_packed) {
+      if (bit_size)
+         validate_assert(state, src->reg.reg->bit_size == bit_size);
+      if (num_components)
+         validate_assert(state, src->reg.reg->num_components == num_components);
+   }
+
    validate_assert(state, (src->reg.reg->num_array_elems == 0 ||
           src->reg.base_offset < src->reg.reg->num_array_elems) &&
           "definitely out-of-bounds array access");
@@ -160,12 +169,13 @@ validate_reg_src(nir_src *src, validate_state *state)
       validate_assert(state, (src->reg.indirect->is_ssa ||
               src->reg.indirect->reg.indirect == NULL) &&
              "only one level of indirection allowed");
-      validate_src(src->reg.indirect, state);
+      validate_src(src->reg.indirect, state, 32, 1);
    }
 }
 
 static void
-validate_ssa_src(nir_src *src, validate_state *state)
+validate_ssa_src(nir_src *src, validate_state *state,
+                 unsigned bit_size, unsigned num_components)
 {
    validate_assert(state, src->ssa != NULL);
 
@@ -188,11 +198,17 @@ validate_ssa_src(nir_src *src, validate_state *state)
       _mesa_set_add(def_state->if_uses, src);
    }
 
+   if (bit_size)
+      validate_assert(state, src->ssa->bit_size == bit_size);
+   if (num_components)
+      validate_assert(state, src->ssa->num_components == num_components);
+
    /* TODO validate that the use is dominated by the definition */
 }
 
 static void
-validate_src(nir_src *src, validate_state *state)
+validate_src(nir_src *src, validate_state *state,
+             unsigned bit_size, unsigned num_components)
 {
    if (state->instr)
       validate_assert(state, src->parent_instr == state->instr);
@@ -200,9 +216,9 @@ validate_src(nir_src *src, validate_state *state)
       validate_assert(state, src->parent_if == state->if_stmt);
 
    if (src->is_ssa)
-      validate_ssa_src(src, state);
+      validate_ssa_src(src, state, bit_size, num_components);
    else
-      validate_reg_src(src, state);
+      validate_reg_src(src, state, bit_size, num_components);
 }
 
 static void
@@ -247,11 +263,12 @@ validate_alu_src(nir_alu_instr *instr, unsigned index, validate_state *state)
       }
    }
 
-   validate_src(&src->src, state);
+   validate_src(&src->src, state, 0, 0);
 }
 
 static void
-validate_reg_dest(nir_reg_dest *dest, validate_state *state)
+validate_reg_dest(nir_reg_dest *dest, validate_state *state,
+                  unsigned bit_size, unsigned num_components)
 {
    validate_assert(state, dest->reg != NULL);
 
@@ -270,6 +287,13 @@ validate_reg_dest(nir_reg_dest *dest, validate_state *state)
              "writing to a register declared in a different function");
    }
 
+   if (!dest->reg->is_packed) {
+      if (bit_size)
+         validate_assert(state, dest->reg->bit_size == bit_size);
+      if (num_components)
+         validate_assert(state, dest->reg->num_components == num_components);
+   }
+
    validate_assert(state, (dest->reg->num_array_elems == 0 ||
           dest->base_offset < dest->reg->num_array_elems) &&
           "definitely out-of-bounds array access");
@@ -278,7 +302,7 @@ validate_reg_dest(nir_reg_dest *dest, validate_state *state)
       validate_assert(state, dest->reg->num_array_elems != 0);
       validate_assert(state, (dest->indirect->is_ssa || dest->indirect->reg.indirect == NULL) &&
              "only one level of indirection allowed");
-      validate_src(dest->indirect, state);
+      validate_src(dest->indirect, state, 32, 1);
    }
 }
 
@@ -307,12 +331,18 @@ validate_ssa_def(nir_ssa_def *def, validate_state *state)
 }
 
 static void
-validate_dest(nir_dest *dest, validate_state *state)
+validate_dest(nir_dest *dest, validate_state *state,
+              unsigned bit_size, unsigned num_components)
 {
-   if (dest->is_ssa)
+   if (dest->is_ssa) {
+      if (bit_size)
+         validate_assert(state, dest->ssa.bit_size == bit_size);
+      if (num_components)
+         validate_assert(state, dest->ssa.num_components == num_components);
       validate_ssa_def(&dest->ssa, state);
-   else
-      validate_reg_dest(&dest->reg, state);
+   } else {
+      validate_reg_dest(&dest->reg, state, bit_size, num_components);
+   }
 }
 
 static void
@@ -350,7 +380,7 @@ validate_alu_dest(nir_alu_instr *instr, validate_state *state)
    validate_assert(state, nir_alu_type_get_type_size(type) == 0 ||
           nir_alu_type_get_type_size(type) == bit_size);
 
-   validate_dest(&dest->dest, state);
+   validate_dest(&dest->dest, state, 0, 0);
 }
 
 static void
@@ -377,7 +407,7 @@ validate_deref_chain(nir_deref *deref, validate_state *state)
          validate_assert(state, deref->type == glsl_get_array_element(parent->type));
          if (nir_deref_as_array(deref)->deref_array_type ==
              nir_deref_array_type_indirect)
-            validate_src(&nir_deref_as_array(deref)->indirect, state);
+            validate_src(&nir_deref_as_array(deref)->indirect, state, 32, 1);
          break;
 
       case nir_deref_type_struct:
@@ -426,6 +456,14 @@ validate_deref_var(void *parent_mem_ctx, nir_deref_var *deref, validate_state *s
 static void
 validate_intrinsic_instr(nir_intrinsic_instr *instr, validate_state *state)
 {
+   unsigned bit_size = 0;
+   if (instr->intrinsic == nir_intrinsic_load_var ||
+       instr->intrinsic == nir_intrinsic_store_var) {
+      const struct glsl_type *type =
+         nir_deref_tail(&instr->variables[0]->deref)->type;
+      bit_size = glsl_get_bit_size(type);
+   }
+
    unsigned num_srcs = nir_intrinsic_infos[instr->intrinsic].num_srcs;
    for (unsigned i = 0; i < num_srcs; i++) {
       unsigned components_read =
@@ -435,13 +473,7 @@ validate_intrinsic_instr(nir_intrinsic_instr *instr, validate_state *state)
 
       validate_assert(state, components_read > 0);
 
-      if (instr->src[i].is_ssa) {
-         validate_assert(state, components_read <= instr->src[i].ssa->num_components);
-      } else if (!instr->src[i].reg.reg->is_packed) {
-         validate_assert(state, components_read <= instr->src[i].reg.reg->num_components);
-      }
-
-      validate_src(&instr->src[i], state);
+      validate_src(&instr->src[i], state, bit_size, components_read);
    }
 
    unsigned num_vars = nir_intrinsic_infos[instr->intrinsic].num_variables;
@@ -457,13 +489,7 @@ validate_intrinsic_instr(nir_intrinsic_instr *instr, validate_state *state)
 
       validate_assert(state, components_written > 0);
 
-      if (instr->dest.is_ssa) {
-         validate_assert(state, components_written <= instr->dest.ssa.num_components);
-      } else if (!instr->dest.reg.reg->is_packed) {
-         validate_assert(state, components_written <= instr->dest.reg.reg->num_components);
-      }
-
-      validate_dest(&instr->dest, state);
+      validate_dest(&instr->dest, state, bit_size, components_written);
    }
 
    switch (instr->intrinsic) {
@@ -511,7 +537,8 @@ validate_tex_instr(nir_tex_instr *instr, validate_state *state)
    for (unsigned i = 0; i < instr->num_srcs; i++) {
       validate_assert(state, !src_type_seen[instr->src[i].src_type]);
       src_type_seen[instr->src[i].src_type] = true;
-      validate_src(&instr->src[i].src, state);
+      validate_src(&instr->src[i].src, state,
+                   0, nir_tex_instr_src_size(instr, i));
    }
 
    if (instr->texture != NULL)
@@ -520,7 +547,7 @@ validate_tex_instr(nir_tex_instr *instr, validate_state *state)
    if (instr->sampler != NULL)
       validate_deref_var(instr, instr->sampler, state);
 
-   validate_dest(&instr->dest, state);
+   validate_dest(&instr->dest, state, 0, nir_tex_instr_dest_size(instr));
 }
 
 static void
@@ -561,7 +588,7 @@ validate_phi_instr(nir_phi_instr *instr, validate_state *state)
     * basic blocks, to avoid validating an SSA use before its definition.
     */
 
-   validate_dest(&instr->dest, state);
+   validate_dest(&instr->dest, state, 0, 0);
 
    exec_list_validate(&instr->srcs);
    validate_assert(state, exec_list_length(&instr->srcs) ==
@@ -626,10 +653,8 @@ validate_phi_src(nir_phi_instr *instr, nir_block *pred, validate_state *state)
    nir_foreach_phi_src(src, instr) {
       if (src->pred == pred) {
          validate_assert(state, src->src.is_ssa);
-         validate_assert(state, src->src.ssa->num_components ==
-                instr->dest.ssa.num_components);
-
-         validate_src(&src->src, state);
+         validate_src(&src->src, state, instr->dest.ssa.bit_size,
+                      instr->dest.ssa.num_components);
          state->instr = NULL;
          return;
       }
@@ -777,7 +802,7 @@ validate_if(nir_if *if_stmt, validate_state *state)
    nir_cf_node *next_node = nir_cf_node_next(&if_stmt->cf_node);
    validate_assert(state, next_node->type == nir_cf_node_block);
 
-   validate_src(&if_stmt->condition, state);
+   validate_src(&if_stmt->condition, state, 32, 1);
 
    validate_assert(state, !exec_list_is_empty(&if_stmt->then_list));
    validate_assert(state, !exec_list_is_empty(&if_stmt->else_list));