[Mesa-dev] [PATCH v2 1/6] nir/validate: Rework ALU bit-size rule validation

[Jason Ekstrand]

The original bit-size validation wasn't capable of properly dealing with
instructions with variable bit sizes.  An attempt was made to handle it
by looking at source and destinations but, because the validation was
done in validate_alu_(src|dest), it didn't really have the needed
information.  The new validation code is much more straightforward and
should be more correct.

Cc: Connor Abbott <cwabbott0 at gmail.com>
Cc: Eric Anholt <eric at anholt.net>
---
 src/compiler/nir/nir_validate.c | 65 +++++++++++++++++++++--------------------
 1 file changed, 33 insertions(+), 32 deletions(-)

diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_validate.c
index 16efcb2..cdbe6a6 100644
--- a/src/compiler/nir/nir_validate.c
+++ b/src/compiler/nir/nir_validate.c
@@ -227,12 +227,9 @@ validate_alu_src(nir_alu_instr *instr, unsigned index, validate_state *state)
    nir_alu_src *src = &instr->src[index];
 
    unsigned num_components;
-   unsigned src_bit_size;
    if (src->src.is_ssa) {
-      src_bit_size = src->src.ssa->bit_size;
       num_components = src->src.ssa->num_components;
    } else {
-      src_bit_size = src->src.reg.reg->bit_size;
       if (src->src.reg.reg->is_packed)
          num_components = 4; /* can't check anything */
       else
@@ -245,24 +242,6 @@ validate_alu_src(nir_alu_instr *instr, unsigned index, validate_state *state)
          validate_assert(state, src->swizzle[i] < num_components);
    }
 
-   nir_alu_type src_type = nir_op_infos[instr->op].input_types[index];
-
-   /* 8-bit float isn't a thing */
-   if (nir_alu_type_get_base_type(src_type) == nir_type_float)
-      validate_assert(state, src_bit_size == 16 || src_bit_size == 32 || src_bit_size == 64);
-
-   if (nir_alu_type_get_type_size(src_type)) {
-      /* This source has an explicit bit size */
-      validate_assert(state, nir_alu_type_get_type_size(src_type) == src_bit_size);
-   } else {
-      if (!nir_alu_type_get_type_size(nir_op_infos[instr->op].output_type)) {
-         unsigned dest_bit_size =
-            instr->dest.dest.is_ssa ? instr->dest.dest.ssa.bit_size
-                                    : instr->dest.dest.reg.reg->bit_size;
-         validate_assert(state, dest_bit_size == src_bit_size);
-      }
-   }
-
    validate_src(&src->src, state, 0, 0);
 }
 
@@ -369,17 +348,6 @@ validate_alu_dest(nir_alu_instr *instr, validate_state *state)
            nir_type_float) ||
           !dest->saturate);
 
-   unsigned bit_size = dest->dest.is_ssa ? dest->dest.ssa.bit_size
-                                         : dest->dest.reg.reg->bit_size;
-   nir_alu_type type = nir_op_infos[instr->op].output_type;
-
-   /* 8-bit float isn't a thing */
-   if (nir_alu_type_get_base_type(type) == nir_type_float)
-      validate_assert(state, bit_size == 16 || bit_size == 32 || bit_size == 64);
-
-   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, 0, 0);
 }
 
@@ -388,10 +356,43 @@ validate_alu_instr(nir_alu_instr *instr, validate_state *state)
 {
    validate_assert(state, instr->op < nir_num_opcodes);
 
+   unsigned instr_bit_size = 0;
    for (unsigned i = 0; i < nir_op_infos[instr->op].num_inputs; i++) {
+      nir_alu_type src_type = nir_op_infos[instr->op].input_types[i];
+      unsigned src_bit_size = nir_src_bit_size(instr->src[i].src);
+      if (nir_alu_type_get_type_size(src_type)) {
+         validate_assert(state, src_bit_size == nir_alu_type_get_type_size(src_type));
+      } else if (instr_bit_size) {
+         validate_assert(state, src_bit_size == instr_bit_size);
+      } else {
+         instr_bit_size = src_bit_size;
+      }
+
+      if (nir_alu_type_get_base_type(src_type) == nir_type_float) {
+         /* 8-bit float isn't a thing */
+         validate_assert(state, src_bit_size == 16 || src_bit_size == 32 ||
+                                src_bit_size == 64);
+      }
+
       validate_alu_src(instr, i, state);
    }
 
+   nir_alu_type dest_type = nir_op_infos[instr->op].output_type;
+   unsigned dest_bit_size = nir_dest_bit_size(instr->dest.dest);
+   if (nir_alu_type_get_type_size(dest_type)) {
+      validate_assert(state, dest_bit_size == nir_alu_type_get_type_size(dest_type));
+   } else if (instr_bit_size) {
+      validate_assert(state, dest_bit_size == instr_bit_size);
+   } else {
+      /* The only unsized thing is the destination so it's vacuously valid */
+   }
+
+   if (nir_alu_type_get_base_type(dest_type) == nir_type_float) {
+      /* 8-bit float isn't a thing */
+      validate_assert(state, dest_bit_size == 16 || dest_bit_size == 32 ||
+                             dest_bit_size == 64);
+   }
+
    validate_alu_dest(instr, state);
 }
 
-- 
2.5.0.400.gff86faf