nir/validate: Rework ALU bit-size rule validation
authorJason Ekstrand <jason.ekstrand@intel.com>
Wed, 8 Mar 2017 18:32:40 +0000 (10:32 -0800)
committerJason Ekstrand <jason.ekstrand@intel.com>
Tue, 14 Mar 2017 14:36:40 +0000 (07:36 -0700)
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.

Reviewed-by: Eric Anholt <eric@anholt.net>
src/compiler/nir/nir_validate.c

index 16efcb2356c0832829f6d0e8e5413f2f8dfd4ef2..cdbe6a6dced2498923babd1b75b4b75d1e5968a0 100644 (file)
@@ -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);
 }