Mesa (master): nir/validate: Validate that bit sizes and components always match

Jason Ekstrand jekstrand at kemper.freedesktop.org
Tue Mar 14 15:56:36 UTC 2017


Module: Mesa
Branch: master
Commit: 370d68babcbb8b2e80fb866429e792ee3c43ffeb
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=370d68babcbb8b2e80fb866429e792ee3c43ffeb

Author: Jason Ekstrand <jason.ekstrand at intel.com>
Date:   Thu Mar  2 16:25:59 2017 -0800

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 at anholt.net>
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
Reviewed-by: Connor Abbott <cwabbott0 at gmail.com>

---

 src/compiler/nir/nir_validate.c | 101 +++++++++++++++++++++++++---------------
 1 file changed, 63 insertions(+), 38 deletions(-)

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




More information about the mesa-commit mailing list