nir: propagate bitsize information in nir_search

When we replace an expresion we have to compute bitsize information for the
replacement. We do this in two passes to validate that bitsize information
is consistent and correct: first we propagate bitsize from child nodes to
parent, then we do it the other way around, starting from the original's
instruction destination bitsize.

v2 (Iago):
- Always use nir_type_bool32 instead of nir_type_bool when generating
  algebraic optimizations. Before we used nir_type_bool32 with constants
  and nir_type_bool with variables.
- Fix bool comparisons in nir_search.c to account for bitsized types.

v3 (Sam):
- Unpack the double constant value as unsigned long long (8 bytes) in
nir_algrebraic.py.

v4 (Sam):
- Use helpers to get type size and base type from nir_alu_type.

Signed-off-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>

src/compiler/nir/nir_algebraic.py

@@ -63,11 +63,11 @@ class Value(object):
 static const ${val.c_type} ${val.name} = {
    { ${val.type_enum} },
 % if isinstance(val, Constant):
-   { ${hex(val)} /* ${val.value} */ },
+   ${val.type()}, { ${hex(val)} /* ${val.value} */ },
 % elif isinstance(val, Variable):
    ${val.index}, /* ${val.var_name} */
    ${'true' if val.is_constant else 'false'},
-   nir_type_${ val.required_type or 'invalid' },
+   ${val.type() or 'nir_type_invalid' },
 % elif isinstance(val, Expression):
    nir_op_${val.opcode},
    { ${', '.join(src.c_ptr for src in val.sources)} },
@@ -107,10 +107,18 @@ class Constant(Value):
       if isinstance(self.value, (int, long)):
          return hex(self.value)
       elif isinstance(self.value, float):
-         return hex(struct.unpack('I', struct.pack('f', self.value))[0])
+         return hex(struct.unpack('Q', struct.pack('d', self.value))[0])
       else:
          assert False
 
+   def type(self):
+      if isinstance(self.value, (bool)):
+         return "nir_type_bool32"
+      elif isinstance(self.value, (int, long)):
+         return "nir_type_int"
+      elif isinstance(self.value, float):
+         return "nir_type_float"
+
 _var_name_re = re.compile(r"(?P<const>#)?(?P<name>\w+)(?:@(?P<type>\w+))?")
 
 class Variable(Value):
@@ -129,6 +137,14 @@ class Variable(Value):
 
       self.index = varset[self.var_name]
 
+   def type(self):
+      if self.required_type == 'bool':
+         return "nir_type_bool32"
+      elif self.required_type in ('int', 'unsigned'):
+         return "nir_type_int"
+      elif self.required_type == 'float':
+         return "nir_type_float"
+
 class Expression(Value):
    def __init__(self, expr, name_base, varset):
       Value.__init__(self, name_base, "expression")

src/compiler/nir/nir_search.c

@@ -62,7 +62,8 @@ alu_instr_is_bool(nir_alu_instr *instr)
    case nir_op_inot:
       return src_is_bool(instr->src[0].src);
    default:
-      return nir_op_infos[instr->op].output_type == nir_type_bool;
+      return (nir_alu_type_get_base_type(nir_op_infos[instr->op].output_type)
+             == nir_type_bool);
    }
 }
 
@@ -125,8 +126,10 @@ match_value(const nir_search_value *value, nir_alu_instr *instr, unsigned src,
             nir_alu_instr *src_alu =
                nir_instr_as_alu(instr->src[src].src.ssa->parent_instr);
 
-            if (nir_op_infos[src_alu->op].output_type != var->type &&
+            if (nir_alu_type_get_base_type(nir_op_infos[src_alu->op].output_type) !=
-                !(var->type == nir_type_bool && alu_instr_is_bool(src_alu)))
+                var->type &&
+                !(nir_alu_type_get_base_type(var->type) == nir_type_bool &&
+                  alu_instr_is_bool(src_alu)))
                return false;
          }
 
@@ -158,21 +161,65 @@ match_value(const nir_search_value *value, nir_alu_instr *instr, unsigned src,
       nir_load_const_instr *load =
          nir_instr_as_load_const(instr->src[src].src.ssa->parent_instr);
 
-      switch (nir_op_infos[instr->op].input_types[src]) {
+      switch (const_val->type) {
       case nir_type_float:
          for (unsigned i = 0; i < num_components; ++i) {
-            if (load->value.f32[new_swizzle[i]] != const_val->data.f)
+            double val;
+            switch (load->def.bit_size) {
+            case 32:
+               val = load->value.f32[new_swizzle[i]];
+               break;
+            case 64:
+               val = load->value.f64[new_swizzle[i]];
+               break;
+            default:
+               unreachable("unknown bit size");
+            }
+
+            if (val != const_val->data.d)
                return false;
          }
          return true;
+
       case nir_type_int:
-      case nir_type_uint:
-      case nir_type_bool:
          for (unsigned i = 0; i < num_components; ++i) {
-            if (load->value.i32[new_swizzle[i]] != const_val->data.i)
+            int64_t val;
+            switch (load->def.bit_size) {
+            case 32:
+               val = load->value.i32[new_swizzle[i]];
+               break;
+            case 64:
+               val = load->value.i64[new_swizzle[i]];
+               break;
+            default:
+               unreachable("unknown bit size");
+            }
+
+            if (val != const_val->data.i)
                return false;
          }
          return true;
+
+      case nir_type_uint:
+      case nir_type_bool32:
+         for (unsigned i = 0; i < num_components; ++i) {
+            uint64_t val;
+            switch (load->def.bit_size) {
+            case 32:
+               val = load->value.u32[new_swizzle[i]];
+               break;
+            case 64:
+               val = load->value.u64[new_swizzle[i]];
+               break;
+            default:
+               unreachable("unknown bit size");
+            }
+
+            if (val != const_val->data.u)
+               return false;
+         }
+         return true;
+
       default:
          unreachable("Invalid alu source type");
       }
@@ -244,9 +291,123 @@ match_expression(const nir_search_expression *expr, nir_alu_instr *instr,
    }
 }
 
+typedef struct bitsize_tree {
+   unsigned num_srcs;
+   struct bitsize_tree *srcs[4];
+
+   unsigned common_size;
+   bool is_src_sized[4];
+   bool is_dest_sized;
+
+   unsigned dest_size;
+   unsigned src_size[4];
+} bitsize_tree;
+
+static bitsize_tree *
+build_bitsize_tree(void *mem_ctx, struct match_state *state,
+                   const nir_search_value *value)
+{
+   bitsize_tree *tree = ralloc(mem_ctx, bitsize_tree);
+
+   switch (value->type) {
+   case nir_search_value_expression: {
+      nir_search_expression *expr = nir_search_value_as_expression(value);
+      nir_op_info info = nir_op_infos[expr->opcode];
+      tree->num_srcs = info.num_inputs;
+      tree->common_size = 0;
+      for (unsigned i = 0; i < info.num_inputs; i++) {
+         tree->is_src_sized[i] = !!nir_alu_type_get_type_size(info.input_types[i]);
+         if (tree->is_src_sized[i])
+            tree->src_size[i] = nir_alu_type_get_type_size(info.input_types[i]);
+         tree->srcs[i] = build_bitsize_tree(mem_ctx, state, expr->srcs[i]);
+      }
+      tree->is_dest_sized = !!nir_alu_type_get_type_size(info.output_type);
+      if (tree->is_dest_sized)
+         tree->dest_size = nir_alu_type_get_type_size(info.output_type);
+      break;
+   }
+
+   case nir_search_value_variable: {
+      nir_search_variable *var = nir_search_value_as_variable(value);
+      tree->num_srcs = 0;
+      tree->is_dest_sized = true;
+      tree->dest_size = nir_src_bit_size(state->variables[var->variable].src);
+      break;
+   }
+
+   case nir_search_value_constant: {
+      tree->num_srcs = 0;
+      tree->is_dest_sized = false;
+      tree->common_size = 0;
+      break;
+   }
+   }
+
+   return tree;
+}
+
+static unsigned
+bitsize_tree_filter_up(bitsize_tree *tree)
+{
+   for (unsigned i = 0; i < tree->num_srcs; i++) {
+      unsigned src_size = bitsize_tree_filter_up(tree->srcs[i]);
+      if (src_size == 0)
+         continue;
+
+      if (tree->is_src_sized[i]) {
+         assert(src_size == tree->src_size[i]);
+      } else if (tree->common_size != 0) {
+         assert(src_size == tree->common_size);
+         tree->src_size[i] = src_size;
+      } else {
+         tree->common_size = src_size;
+         tree->src_size[i] = src_size;
+      }
+   }
+
+   if (tree->num_srcs && tree->common_size) {
+      if (tree->dest_size == 0)
+         tree->dest_size = tree->common_size;
+      else if (!tree->is_dest_sized)
+         assert(tree->dest_size == tree->common_size);
+
+      for (unsigned i = 0; i < tree->num_srcs; i++) {
+         if (!tree->src_size[i])
+            tree->src_size[i] = tree->common_size;
+      }
+   }
+
+   return tree->dest_size;
+}
+
+static void
+bitsize_tree_filter_down(bitsize_tree *tree, unsigned size)
+{
+   if (tree->dest_size)
+      assert(tree->dest_size == size);
+   else
+      tree->dest_size = size;
+
+   if (!tree->is_dest_sized) {
+      if (tree->common_size)
+         assert(tree->common_size == size);
+      else
+         tree->common_size = size;
+   }
+
+   for (unsigned i = 0; i < tree->num_srcs; i++) {
+      if (!tree->src_size[i]) {
+         assert(tree->common_size);
+         tree->src_size[i] = tree->common_size;
+      }
+      bitsize_tree_filter_down(tree->srcs[i], tree->src_size[i]);
+   }
+}
+
 static nir_alu_src
-construct_value(const nir_search_value *value, nir_alu_type type,
+construct_value(const nir_search_value *value,
-                unsigned num_components, struct match_state *state,
+                unsigned num_components, bitsize_tree *bitsize,
+                struct match_state *state,
                 nir_instr *instr, void *mem_ctx)
 {
    switch (value->type) {
@@ -257,7 +418,8 @@ construct_value(const nir_search_value *value, nir_alu_type type,
          num_components = nir_op_infos[expr->opcode].output_size;
 
       nir_alu_instr *alu = nir_alu_instr_create(mem_ctx, expr->opcode);
-      nir_ssa_dest_init(&alu->instr, &alu->dest.dest, num_components, 32, NULL);
+      nir_ssa_dest_init(&alu->instr, &alu->dest.dest, num_components,
+                        bitsize->dest_size, NULL);
       alu->dest.write_mask = (1 << num_components) - 1;
       alu->dest.saturate = false;
 
@@ -269,8 +431,7 @@ construct_value(const nir_search_value *value, nir_alu_type type,
             num_components = nir_op_infos[alu->op].input_sizes[i];
 
          alu->src[i] = construct_value(expr->srcs[i],
-                                       nir_op_infos[alu->op].input_types[i],
+                                       num_components, bitsize->srcs[i],
-                                       num_components,
                                        state, instr, mem_ctx);
       }
 
@@ -301,23 +462,57 @@ construct_value(const nir_search_value *value, nir_alu_type type,
       const nir_search_constant *c = nir_search_value_as_constant(value);
       nir_load_const_instr *load = nir_load_const_instr_create(mem_ctx, 1);
 
-      switch (type) {
+      switch (c->type) {
       case nir_type_float:
-         load->def.name = ralloc_asprintf(mem_ctx, "%f", c->data.f);
+         load->def.name = ralloc_asprintf(mem_ctx, "%f", c->data.d);
-         load->value.f32[0] = c->data.f;
+         switch (bitsize->dest_size) {
+         case 32:
+            load->value.f32[0] = c->data.d;
             break;
+         case 64:
+            load->value.f64[0] = c->data.d;
+            break;
+         default:
+            unreachable("unknown bit size");
+         }
+         break;
+
       case nir_type_int:
-         load->def.name = ralloc_asprintf(mem_ctx, "%d", c->data.i);
+         load->def.name = ralloc_asprintf(mem_ctx, "%ld", c->data.i);
+         switch (bitsize->dest_size) {
+         case 32:
             load->value.i32[0] = c->data.i;
             break;
+         case 64:
+            load->value.i64[0] = c->data.i;
+            break;
+         default:
+            unreachable("unknown bit size");
+         }
+         break;
+
       case nir_type_uint:
-      case nir_type_bool:
+         load->def.name = ralloc_asprintf(mem_ctx, "%lu", c->data.u);
+         switch (bitsize->dest_size) {
+         case 32:
+            load->value.u32[0] = c->data.u;
+            break;
+         case 64:
+            load->value.u64[0] = c->data.u;
+            break;
+         default:
+            unreachable("unknown bit size");
+         }
+
+      case nir_type_bool32:
          load->value.u32[0] = c->data.u;
          break;
       default:
          unreachable("Invalid alu source type");
       }
 
+      load->def.bit_size = bitsize->dest_size;
+
       nir_instr_insert_before(instr, &load->instr);
 
       nir_alu_src val;
@@ -352,6 +547,11 @@ nir_replace_instr(nir_alu_instr *instr, const nir_search_expression *search,
                          swizzle, &state))
       return NULL;
 
+   void *bitsize_ctx = ralloc_context(NULL);
+   bitsize_tree *tree = build_bitsize_tree(bitsize_ctx, &state, replace);
+   bitsize_tree_filter_up(tree);
+   bitsize_tree_filter_down(tree, instr->dest.dest.ssa.bit_size);
+
    /* Inserting a mov may be unnecessary.  However, it's much easier to
     * simply let copy propagation clean this up than to try to go through
     * and rewrite swizzles ourselves.
@@ -362,9 +562,9 @@ nir_replace_instr(nir_alu_instr *instr, const nir_search_expression *search,
                      instr->dest.dest.ssa.num_components,
                      instr->dest.dest.ssa.bit_size, NULL);
 
-   mov->src[0] = construct_value(replace, nir_op_infos[instr->op].output_type,
+   mov->src[0] = construct_value(replace,
-                                 instr->dest.dest.ssa.num_components, &state,
+                                 instr->dest.dest.ssa.num_components,
-                                 &instr->instr, mem_ctx);
+                                 tree, &state, &instr->instr, mem_ctx);
    nir_instr_insert_before(&instr->instr, &mov->instr);
 
    nir_ssa_def_rewrite_uses(&instr->dest.dest.ssa,
@@ -376,5 +576,7 @@ nir_replace_instr(nir_alu_instr *instr, const nir_search_expression *search,
     */
    nir_instr_remove(&instr->instr);
 
+   ralloc_free(bitsize_ctx);
+
    return mov;
 }

src/compiler/nir/nir_search.h

@@ -71,10 +71,12 @@ typedef struct {
 typedef struct {
    nir_search_value value;
 
+   nir_alu_type type;
+
    union {
-      uint32_t u;
+      uint64_t u;
-      int32_t i;
+      int64_t i;
-      float f;
+      double d;
    } data;
 } nir_search_constant;