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 <[email protected]>
Reviewed-by: Kenneth Graunke <[email protected]>
Reviewed-by: Connor Abbott <[email protected]>

1 files changed, 63 insertions, 38 deletions

diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_validate.c
index 15ba65f2b12..16efcb2356c 100644
--- 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));