[Mesa-dev] [RFC 7/9] nir/nir: Use a linked list instead of a has set for use/def sets

Connor Abbott cwabbott0 at gmail.com
Mon Apr 27 13:35:10 PDT 2015


On Fri, Apr 24, 2015 at 7:32 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> This commit switches us from the current setup of using hash sets for
> use/def sets to using linked lists.  Doing so should save us quite a bit of
> memory because we aren't carrying around 3 hash sets per register and 2 per
> SSA value.  It should also save us CPU time because adding/removing things
> from use/def sets is 4 pointer manipulations instead of a hash lookup.
>
> On the code complexity side of things, some things are now much easier and
> others are a bit harder.  One of the operations we perform constantly in
> optimization passes is to replace one source with another.  Due to the fact
> that an instruction can use the same SSA value multiple times, we had to
> iterate through the sources of the instruction and determine if the use we
> were replacing was the only one before removing it from the set of uses.
> With this patch, uses are per-source not per-instruction so we can just
> remove it safely.  On the other hand, trying to iterate over all of the
> instructions that use a given value is more difficult.  Fortunately, the
> two places we do that are the ffma peephole where it doesn't matter and GCM
> where we already gracefully handle duplicates visits to an instruction.
>
> Another aspect here is that using linked lists in this way can be tricky to
> get right.  With sets, things were quite forgiving and the worst that
> happened if you didn't properly remove a use was that it would get caught
> in the validator.  With linked lists, it can lead to linked list corruption
> which can be harder to track.  However, we do just as much validation of
> the linked lists as we did of the sets so the validator should still catch
> these problems.  While working on this series, the vast majority of the
> bugs I had to fix were caught by assertions.  I don't think the lists are
> going to be that much worse than the sets.
> ---
>  src/glsl/nir/nir.c          | 232 ++++++++++++++++----------------------------
>  src/glsl/nir/nir.h          |  27 ++++--
>  src/glsl/nir/nir_validate.c | 158 +++++++++++++++---------------
>  3 files changed, 182 insertions(+), 235 deletions(-)
>
> diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
> index b8f5dd4..283b861 100644
> --- a/src/glsl/nir/nir.c
> +++ b/src/glsl/nir/nir.c
> @@ -58,12 +58,9 @@ reg_create(void *mem_ctx, struct exec_list *list)
>     nir_register *reg = ralloc(mem_ctx, nir_register);
>
>     reg->parent_instr = NULL;
> -   reg->uses = _mesa_set_create(reg, _mesa_hash_pointer,
> -                                _mesa_key_pointer_equal);
> -   reg->defs = _mesa_set_create(reg, _mesa_hash_pointer,
> -                                _mesa_key_pointer_equal);
> -   reg->if_uses = _mesa_set_create(reg, _mesa_hash_pointer,
> -                                   _mesa_key_pointer_equal);
> +   nir_list_init(&reg->uses);
> +   nir_list_init(&reg->defs);
> +   nir_list_init(&reg->if_uses);
>
>     reg->num_components = 0;
>     reg->num_array_elems = 0;
> @@ -1070,11 +1067,14 @@ update_if_uses(nir_cf_node *node)
>
>     nir_if *if_stmt = nir_cf_node_as_if(node);
>
> -   struct set *if_uses_set = if_stmt->condition.is_ssa ?
> -                             if_stmt->condition.ssa->if_uses :
> -                             if_stmt->condition.reg.reg->uses;
> -
> -   _mesa_set_add(if_uses_set, if_stmt);
> +   if_stmt->condition.parent_if = if_stmt;
> +   if (if_stmt->condition.is_ssa) {
> +      nir_list_push_tail(&if_stmt->condition.ssa->if_uses,
> +                         &if_stmt->condition.use_link);
> +   } else {
> +      nir_list_push_tail(&if_stmt->condition.reg.reg->if_uses,
> +                         &if_stmt->condition.use_link);
> +   }
>  }
>
>  void
> @@ -1227,16 +1227,7 @@ cleanup_cf_node(nir_cf_node *node)
>        foreach_list_typed(nir_cf_node, child, node, &if_stmt->else_list)
>           cleanup_cf_node(child);
>
> -      struct set *if_uses;
> -      if (if_stmt->condition.is_ssa) {
> -         if_uses = if_stmt->condition.ssa->if_uses;
> -      } else {
> -         if_uses = if_stmt->condition.reg.reg->if_uses;
> -      }
> -
> -      struct set_entry *entry = _mesa_set_search(if_uses, if_stmt);
> -      assert(entry);
> -      _mesa_set_remove(if_uses, entry);
> +      nir_link_remove(&if_stmt->condition.use_link);
>        break;
>     }
>
> @@ -1293,9 +1284,10 @@ add_use_cb(nir_src *src, void *state)
>  {
>     nir_instr *instr = state;
>
> -   struct set *uses_set = src->is_ssa ? src->ssa->uses : src->reg.reg->uses;
> -
> -   _mesa_set_add(uses_set, instr);
> +   src->parent_instr = instr;
> +   nir_link_init(&src->use_link);
> +   nir_list *uses_list = src->is_ssa ? &src->ssa->uses : &src->reg.reg->uses;
> +   nir_list_push_tail(uses_list, &src->use_link);
>
>     return true;
>  }
> @@ -1320,8 +1312,11 @@ add_reg_def_cb(nir_dest *dest, void *state)
>  {
>     nir_instr *instr = state;
>
> -   if (!dest->is_ssa)
> -      _mesa_set_add(dest->reg.reg->defs, instr);
> +   if (!dest->is_ssa) {
> +      dest->reg.parent_instr = instr;
> +      nir_link_init(&dest->reg.def_link);
> +      nir_list_push_tail(&dest->reg.reg->defs, &dest->reg.def_link);
> +   }
>
>     return true;
>  }
> @@ -1436,13 +1431,7 @@ nir_instr_insert_after_cf_list(struct exec_list *list, nir_instr *after)
>  static bool
>  remove_use_cb(nir_src *src, void *state)
>  {
> -   nir_instr *instr = state;
> -
> -   struct set *uses_set = src->is_ssa ? src->ssa->uses : src->reg.reg->uses;
> -
> -   struct set_entry *entry = _mesa_set_search(uses_set, instr);
> -   if (entry)
> -      _mesa_set_remove(uses_set, entry);
> +   nir_link_remove(&src->use_link);
>
>     return true;
>  }
> @@ -1450,16 +1439,8 @@ remove_use_cb(nir_src *src, void *state)
>  static bool
>  remove_def_cb(nir_dest *dest, void *state)
>  {
> -   nir_instr *instr = state;
> -
> -   if (dest->is_ssa)
> -      return true;
> -
> -   nir_register *reg = dest->reg.reg;
> -
> -   struct set_entry *entry = _mesa_set_search(reg->defs, instr);
> -   if (entry)
> -      _mesa_set_remove(reg->defs, entry);
> +   if (!dest->is_ssa)
> +      nir_link_remove(&dest->reg.def_link);
>
>     return true;
>  }
> @@ -1834,86 +1815,71 @@ nir_srcs_equal(nir_src src1, nir_src src2)
>  }
>
>  static bool
> -src_does_not_use_def(nir_src *src, void *void_def)
> +src_is_valid(const nir_src *src)
>  {
> -   nir_ssa_def *def = void_def;
> -
> -   if (src->is_ssa) {
> -      return src->ssa != def;
> -   } else {
> -      return true;
> -   }
> +   return src->is_ssa ? (src->ssa != NULL) : (src->reg.reg != NULL);
>  }
>
> -static bool
> -src_does_not_use_reg(nir_src *src, void *void_reg)
> +static void
> +src_remove_all_uses(nir_src *src)
>  {
> -   nir_register *reg = void_reg;
> +   for (; src; src = src->is_ssa ? NULL : src->reg.indirect) {
> +      if (!src_is_valid(src))
> +         continue;
>
> -   if (src->is_ssa) {
> -      return true;
> -   } else {
> -      return src->reg.reg != reg;
> +      nir_link_remove(&src->use_link);
>     }
>  }
>
> -void
> -nir_instr_rewrite_src(nir_instr *instr, nir_src *src, nir_src new_src)
> +static void
> +src_add_all_uses(nir_src *src, nir_instr *parent_instr, nir_if *parent_if)
>  {
> -   nir_src old_src = *src;
> -   *src = new_src;
> +   for (; src; src = src->is_ssa ? NULL : src->reg.indirect) {
> +      if (!src_is_valid(src))
> +         continue;
>
> -   for (nir_src *iter_src = &old_src; iter_src;
> -        iter_src = iter_src->is_ssa ? NULL : iter_src->reg.indirect) {
> -      if (iter_src->is_ssa) {
> -         nir_ssa_def *ssa = iter_src->ssa;
> -         if (ssa && nir_foreach_src(instr, src_does_not_use_def, ssa)) {
> -            struct set_entry *entry = _mesa_set_search(ssa->uses, instr);
> -            assert(entry);
> -            _mesa_set_remove(ssa->uses, entry);
> -         }
> +      /* This source may have been mostly uninitialized or have come from
> +       * another instruction.  In either of these cases, the parent field
> +       * and list link will be uninitialized so we initialize them here.
> +       */
> +      nir_link_init(&src->use_link);
> +
> +      if (parent_instr) {
> +         src->parent_instr = parent_instr;
> +         if (src->is_ssa)
> +            nir_list_push_tail(&src->ssa->uses, &src->use_link);
> +         else
> +            nir_list_push_tail(&src->reg.reg->uses, &src->use_link);
>        } else {
> -         nir_register *reg = iter_src->reg.reg;
> -         if (reg && nir_foreach_src(instr, src_does_not_use_reg, reg)) {
> -            struct set_entry *entry = _mesa_set_search(reg->uses, instr);
> -            assert(entry);
> -            _mesa_set_remove(reg->uses, entry);
> -         }
> +         assert(parent_if);
> +         src->parent_if = parent_if;
> +         if (src->is_ssa)
> +            nir_list_push_tail(&src->ssa->if_uses, &src->use_link);
> +         else
> +            nir_list_push_tail(&src->reg.reg->if_uses, &src->use_link);
>        }
>     }
> +}
>
> -   for (nir_src *iter_src = &new_src; iter_src;
> -        iter_src = iter_src->is_ssa ? NULL : iter_src->reg.indirect) {
> -      if (iter_src->is_ssa) {
> -         if (iter_src->ssa)
> -            _mesa_set_add(iter_src->ssa->uses, instr);
> -      } else {
> -         if (iter_src->reg.reg)
> -            _mesa_set_add(iter_src->reg.reg->uses, instr);
> -      }
> -   }
> +void
> +nir_instr_rewrite_src(nir_instr *instr, nir_src *src, nir_src new_src)
> +{
> +   assert(!src_is_valid(src) || src->parent_instr == instr);
> +
> +   src_remove_all_uses(src);
> +   *src = new_src;
> +   src_add_all_uses(src, instr, NULL);
>  }
>
>  void
>  nir_if_rewrite_condition(nir_if *if_stmt, nir_src new_src)
>  {
> -   for (nir_src *src = &if_stmt->condition; src;
> -        src = src->is_ssa ? NULL : src->reg.indirect) {
> -      struct set *uses = src->is_ssa ? src->ssa->if_uses
> -                                     : src->reg.reg->if_uses;
> -      struct set_entry *entry = _mesa_set_search(uses, if_stmt);
> -      assert(entry);
> -      _mesa_set_remove(uses, entry);
> -   }
> -
> -   if_stmt->condition = new_src;
> +   nir_src *src = &if_stmt->condition;
> +   assert(!src_is_valid(src) || src->parent_if == if_stmt);
>
> -   for (nir_src *src = &if_stmt->condition; src;
> -        src = src->is_ssa ? NULL : src->reg.indirect) {
> -      struct set *uses = src->is_ssa ? src->ssa->if_uses
> -                                     : src->reg.reg->if_uses;
> -      _mesa_set_add(uses, if_stmt);
> -   }
> +   src_remove_all_uses(src);
> +   *src = new_src;
> +   src_add_all_uses(src, NULL, if_stmt);
>  }
>
>  void
> @@ -1922,10 +1888,8 @@ nir_ssa_def_init(nir_instr *instr, nir_ssa_def *def,
>  {
>     def->name = name;
>     def->parent_instr = instr;
> -   def->uses = _mesa_set_create(instr, _mesa_hash_pointer,
> -                                _mesa_key_pointer_equal);
> -   def->if_uses = _mesa_set_create(instr, _mesa_hash_pointer,
> -                                   _mesa_key_pointer_equal);
> +   nir_list_init(&def->uses);
> +   nir_list_init(&def->if_uses);
>     def->num_components = num_components;
>
>     if (instr->block) {
> @@ -1946,57 +1910,23 @@ nir_ssa_dest_init(nir_instr *instr, nir_dest *dest,
>     nir_ssa_def_init(instr, &dest->ssa, num_components, name);
>  }
>
> -struct ssa_def_rewrite_state {
> -   void *mem_ctx;
> -   nir_ssa_def *old;
> -   nir_src new_src;
> -};
> -
> -static bool
> -ssa_def_rewrite_uses_src(nir_src *src, void *void_state)
> -{
> -   struct ssa_def_rewrite_state *state = void_state;
> -
> -   if (src->is_ssa && src->ssa == state->old)
> -      nir_src_copy(src, &state->new_src, state->mem_ctx);
> -
> -   return true;
> -}
> -
>  void
>  nir_ssa_def_rewrite_uses(nir_ssa_def *def, nir_src new_src, void *mem_ctx)
>  {
> -   struct ssa_def_rewrite_state state;
> -   state.mem_ctx = mem_ctx;
> -   state.old = def;
> -   state.new_src = new_src;
> -
>     assert(!new_src.is_ssa || def != new_src.ssa);
>
> -   struct set *new_uses, *new_if_uses;
> -   if (new_src.is_ssa) {
> -      new_uses = new_src.ssa->uses;
> -      new_if_uses = new_src.ssa->if_uses;
> -   } else {
> -      new_uses = new_src.reg.reg->uses;
> -      new_if_uses = new_src.reg.reg->if_uses;
> -   }
> -
> -   struct set_entry *entry;
> -   set_foreach(def->uses, entry) {
> -      nir_instr *instr = (nir_instr *)entry->key;
> -
> -      _mesa_set_remove(def->uses, entry);
> -      nir_foreach_src(instr, ssa_def_rewrite_uses_src, &state);
> -      _mesa_set_add(new_uses, instr);
> +   nir_list_foreach_safe(nir_src, use_src, use_link, &def->uses) {
> +      nir_instr *src_parent_instr = use_src->parent_instr;
> +      nir_link_remove(&use_src->use_link);
> +      nir_src_copy(use_src, &new_src, mem_ctx);
> +      src_add_all_uses(use_src, src_parent_instr, NULL);
>     }
>
> -   set_foreach(def->if_uses, entry) {
> -      nir_if *if_use = (nir_if *)entry->key;
> -
> -      _mesa_set_remove(def->if_uses, entry);
> -      nir_src_copy(&if_use->condition, &new_src, mem_ctx);
> -      _mesa_set_add(new_if_uses, if_use);
> +   nir_list_foreach_safe(nir_src, use_src, use_link, &def->if_uses) {
> +      nir_if *src_parent_if = use_src->parent_if;
> +      nir_link_remove(&use_src->use_link);
> +      nir_src_copy(use_src, &new_src, mem_ctx);
> +      src_add_all_uses(use_src, NULL, src_parent_if);
>     }
>  }
>
> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> index aaf1c57..364071d 100644
> --- a/src/glsl/nir/nir.h
> +++ b/src/glsl/nir/nir.h
> @@ -37,6 +37,7 @@
>  #include "glsl/shader_enums.h"
>  #include <stdio.h>
>
> +#include "nir_list.h"
>  #include "nir_opcodes.h"
>
>  #ifdef __cplusplus
> @@ -397,13 +398,13 @@ typedef struct {
>     struct nir_instr *parent_instr;
>
>     /** set of nir_instr's where this register is used (read from) */
> -   struct set *uses;
> +   nir_list uses;
>
>     /** set of nir_instr's where this register is defined (written to) */
> -   struct set *defs;
> +   nir_list defs;
>
>     /** set of nir_if's where this register is used as a condition */
> -   struct set *if_uses;
> +   nir_list if_uses;
>  } nir_register;
>
>  typedef enum {
> @@ -462,10 +463,10 @@ typedef struct {
>     nir_instr *parent_instr;
>
>     /** set of nir_instr's where this register is used (read from) */
> -   struct set *uses;
> +   nir_list uses;
>
>     /** set of nir_if's where this register is used as a condition */
> -   struct set *if_uses;
> +   nir_list if_uses;
>
>     uint8_t num_components;
>  } nir_ssa_def;
> @@ -481,6 +482,9 @@ typedef struct {
>  } nir_reg_src;
>
>  typedef struct {
> +   nir_instr *parent_instr;
> +   nir_link def_link;
> +
>     nir_register *reg;
>     struct nir_src *indirect; /** < NULL for no indirect offset */
>     unsigned base_offset;
> @@ -488,8 +492,17 @@ typedef struct {
>     /* TODO def-use chain goes here */
>  } nir_reg_dest;
>
> +struct nir_if;
> +
>  typedef struct nir_src {
>     union {
> +      nir_instr *parent_instr;
> +      struct nir_if *parent_if;
> +   };

There's something I'm not quite understanding about this... how are we
supposed to know which of parent_instr and parent_if are valid? If I
walk over all the sources for a given SSA def or register, how am I
supposed to know if it's part of an if-condition or an instruction? I
would think that you would need a boolean here or have parent_instr
and parent_if not be in a union.

> +
> +   nir_link use_link;
> +
> +   union {
>        nir_reg_src reg;
>        nir_ssa_def *ssa;
>     };
> @@ -497,7 +510,7 @@ typedef struct nir_src {
>     bool is_ssa;
>  } nir_src;
>
> -#define NIR_SRC_INIT (nir_src) { { { NULL } } }
> +#define NIR_SRC_INIT (nir_src) { { NULL } }
>
>  typedef struct {
>     union {
> @@ -1208,7 +1221,7 @@ nir_block_last_instr(nir_block *block)
>  #define nir_foreach_instr_safe(block, instr) \
>     foreach_list_typed_safe(nir_instr, instr, node, &(block)->instr_list)
>
> -typedef struct {
> +typedef struct nir_if {
>     nir_cf_node cf_node;
>     nir_src condition;
>
> diff --git a/src/glsl/nir/nir_validate.c b/src/glsl/nir/nir_validate.c
> index 35a853d..97010f2 100644
> --- a/src/glsl/nir/nir_validate.c
> +++ b/src/glsl/nir/nir_validate.c
> @@ -97,50 +97,47 @@ typedef struct {
>  static void validate_src(nir_src *src, validate_state *state);
>
>  static void
> -validate_reg_src(nir_reg_src *src, validate_state *state)
> +validate_reg_src(nir_src *src, validate_state *state)
>  {
> -   assert(src->reg != NULL);
> +   assert(src->reg.reg != NULL);
>
>     struct hash_entry *entry;
> -   entry = _mesa_hash_table_search(state->regs, src->reg);
> +   entry = _mesa_hash_table_search(state->regs, src->reg.reg);
>     assert(entry);
>
>     reg_validate_state *reg_state = (reg_validate_state *) entry->data;
>
>     if (state->instr) {
> -      _mesa_set_add(reg_state->uses, state->instr);
> -
> -      assert(_mesa_set_search(src->reg->uses, state->instr));
> +      _mesa_set_add(reg_state->uses, src);
>     } else {
>        assert(state->if_stmt);
> -      _mesa_set_add(reg_state->if_uses, state->if_stmt);
> -
> -      assert(_mesa_set_search(src->reg->if_uses, state->if_stmt));
> +      _mesa_set_add(reg_state->if_uses, src);
>     }
>
> -   if (!src->reg->is_global) {
> +   if (!src->reg.reg->is_global) {
>        assert(reg_state->where_defined == state->impl &&
>               "using a register declared in a different function");
>     }
>
> -   assert((src->reg->num_array_elems == 0 ||
> -          src->base_offset < src->reg->num_array_elems) &&
> +   assert((src->reg.reg->num_array_elems == 0 ||
> +          src->reg.base_offset < src->reg.reg->num_array_elems) &&
>            "definitely out-of-bounds array access");
>
> -   if (src->indirect) {
> -      assert(src->reg->num_array_elems != 0);
> -      assert((src->indirect->is_ssa || src->indirect->reg.indirect == NULL) &&
> +   if (src->reg.indirect) {
> +      assert(src->reg.reg->num_array_elems != 0);
> +      assert((src->reg.indirect->is_ssa ||
> +              src->reg.indirect->reg.indirect == NULL) &&
>               "only one level of indirection allowed");
> -      validate_src(src->indirect, state);
> +      validate_src(src->reg.indirect, state);
>     }
>  }
>
>  static void
> -validate_ssa_src(nir_ssa_def *def, validate_state *state)
> +validate_ssa_src(nir_src *src, validate_state *state)
>  {
> -   assert(def != NULL);
> +   assert(src->ssa != NULL);
>
> -   struct hash_entry *entry = _mesa_hash_table_search(state->ssa_defs, def);
> +   struct hash_entry *entry = _mesa_hash_table_search(state->ssa_defs, src->ssa);
>
>     assert(entry);
>
> @@ -150,14 +147,10 @@ validate_ssa_src(nir_ssa_def *def, validate_state *state)
>            "using an SSA value defined in a different function");
>
>     if (state->instr) {
> -      _mesa_set_add(def_state->uses, state->instr);
> -
> -      assert(_mesa_set_search(def->uses, state->instr));
> +      _mesa_set_add(def_state->uses, src);
>     } else {
>        assert(state->if_stmt);
> -      _mesa_set_add(def_state->if_uses, state->if_stmt);
> -
> -      assert(_mesa_set_search(def->if_uses, state->if_stmt));
> +      _mesa_set_add(def_state->if_uses, src);
>     }
>
>     /* TODO validate that the use is dominated by the definition */
> @@ -166,10 +159,15 @@ validate_ssa_src(nir_ssa_def *def, validate_state *state)
>  static void
>  validate_src(nir_src *src, validate_state *state)
>  {
> +   if (state->instr)
> +      assert(src->parent_instr == state->instr);
> +   else
> +      assert(src->parent_if == state->if_stmt);
> +
>     if (src->is_ssa)
> -      validate_ssa_src(src->ssa, state);
> +      validate_ssa_src(src, state);
>     else
> -      validate_reg_src(&src->reg, state);
> +      validate_reg_src(src, state);
>  }
>
>  static void
> @@ -201,8 +199,7 @@ validate_reg_dest(nir_reg_dest *dest, validate_state *state)
>  {
>     assert(dest->reg != NULL);
>
> -   struct set_entry *entry = _mesa_set_search(dest->reg->defs, state->instr);
> -   assert(entry && "definition not in nir_register.defs");
> +   assert(dest->parent_instr == state->instr);
>
>     struct hash_entry *entry2;
>     entry2 = _mesa_hash_table_search(state->regs, dest->reg);
> @@ -210,7 +207,7 @@ validate_reg_dest(nir_reg_dest *dest, validate_state *state)
>     assert(entry2);
>
>     reg_validate_state *reg_state = (reg_validate_state *) entry2->data;
> -   _mesa_set_add(reg_state->defs, state->instr);
> +   _mesa_set_add(reg_state->defs, dest);
>
>     if (!dest->reg->is_global) {
>        assert(reg_state->where_defined == state->impl &&
> @@ -240,6 +237,9 @@ validate_ssa_def(nir_ssa_def *def, validate_state *state)
>
>     assert(def->num_components <= 4);
>
> +   nir_list_validate(&def->uses);
> +   nir_list_validate(&def->if_uses);
> +
>     ssa_def_validate_state *def_state = ralloc(state->ssa_defs,
>                                                ssa_def_validate_state);
>     def_state->where_defined = state->impl;
> @@ -701,6 +701,10 @@ prevalidate_reg_decl(nir_register *reg, bool is_global, validate_state *state)
>     assert(!BITSET_TEST(state->regs_found, reg->index));
>     BITSET_SET(state->regs_found, reg->index);
>
> +   nir_list_validate(&reg->uses);
> +   nir_list_validate(&reg->defs);
> +   nir_list_validate(&reg->if_uses);
> +
>     reg_validate_state *reg_state = ralloc(state->regs, reg_validate_state);
>     reg_state->uses = _mesa_set_create(reg_state, _mesa_hash_pointer,
>                                        _mesa_key_pointer_equal);
> @@ -721,47 +725,47 @@ postvalidate_reg_decl(nir_register *reg, validate_state *state)
>
>     reg_validate_state *reg_state = (reg_validate_state *) entry->data;
>
> -   if (reg_state->uses->entries != reg->uses->entries) {
> +   nir_list_foreach(nir_src, src, use_link, &reg->uses) {
> +      struct set_entry *entry = _mesa_set_search(reg_state->uses, src);
> +      assert(entry);
> +      _mesa_set_remove(reg_state->uses, entry);
> +   }
> +
> +   if (reg_state->uses->entries != 0) {
>        printf("extra entries in register uses:\n");
>        struct set_entry *entry;
> -      set_foreach(reg->uses, entry) {
> -         struct set_entry *entry2 =
> -            _mesa_set_search(reg_state->uses, entry->key);
> -
> -         if (entry2 == NULL) {
> -            printf("%p\n", entry->key);
> -         }
> -      }
> +      set_foreach(reg_state->uses, entry)
> +         printf("%p\n", entry->key);
>
>        abort();
>     }
>
> -   if (reg_state->if_uses->entries != reg->if_uses->entries) {
> +   nir_list_foreach(nir_src, src, use_link, &reg->if_uses) {
> +      struct set_entry *entry = _mesa_set_search(reg_state->if_uses, src);
> +      assert(entry);
> +      _mesa_set_remove(reg_state->if_uses, entry);
> +   }
> +
> +   if (reg_state->if_uses->entries != 0) {
>        printf("extra entries in register if_uses:\n");
>        struct set_entry *entry;
> -      set_foreach(reg->if_uses, entry) {
> -         struct set_entry *entry2 =
> -            _mesa_set_search(reg_state->if_uses, entry->key);
> -
> -         if (entry2 == NULL) {
> -            printf("%p\n", entry->key);
> -         }
> -      }
> +      set_foreach(reg_state->if_uses, entry)
> +         printf("%p\n", entry->key);
>
>        abort();
>     }
>
> -   if (reg_state->defs->entries != reg->defs->entries) {
> +   nir_list_foreach(nir_src, src, use_link, &reg->defs) {
> +      struct set_entry *entry = _mesa_set_search(reg_state->defs, src);
> +      assert(entry);
> +      _mesa_set_remove(reg_state->defs, entry);
> +   }
> +
> +   if (reg_state->defs->entries != 0) {
>        printf("extra entries in register defs:\n");
>        struct set_entry *entry;
> -      set_foreach(reg->defs, entry) {
> -         struct set_entry *entry2 =
> -            _mesa_set_search(reg_state->defs, entry->key);
> -
> -         if (entry2 == NULL) {
> -            printf("%p\n", entry->key);
> -         }
> -      }
> +      set_foreach(reg_state->defs, entry)
> +         printf("%p\n", entry->key);
>
>        abort();
>     }
> @@ -790,32 +794,32 @@ postvalidate_ssa_def(nir_ssa_def *def, void *void_state)
>     struct hash_entry *entry = _mesa_hash_table_search(state->ssa_defs, def);
>     ssa_def_validate_state *def_state = (ssa_def_validate_state *)entry->data;
>
> -   if (def_state->uses->entries != def->uses->entries) {
> -      printf("extra entries in SSA def uses:\n");
> -      struct set_entry *entry;
> -      set_foreach(def->uses, entry) {
> -         struct set_entry *entry2 =
> -            _mesa_set_search(def_state->uses, entry->key);
> +   nir_list_foreach(nir_src, src, use_link, &def->uses) {
> +      struct set_entry *entry = _mesa_set_search(def_state->uses, src);
> +      assert(entry);
> +      _mesa_set_remove(def_state->uses, entry);
> +   }
>
> -         if (entry2 == NULL) {
> -            printf("%p\n", entry->key);
> -         }
> -      }
> +   if (def_state->uses->entries != 0) {
> +      printf("extra entries in register uses:\n");
> +      struct set_entry *entry;
> +      set_foreach(def_state->uses, entry)
> +         printf("%p\n", entry->key);
>
>        abort();
>     }
>
> -   if (def_state->if_uses->entries != def->if_uses->entries) {
> -      printf("extra entries in SSA def uses:\n");
> -      struct set_entry *entry;
> -      set_foreach(def->if_uses, entry) {
> -         struct set_entry *entry2 =
> -            _mesa_set_search(def_state->if_uses, entry->key);
> +   nir_list_foreach(nir_src, src, use_link, &def->if_uses) {
> +      struct set_entry *entry = _mesa_set_search(def_state->if_uses, src);
> +      assert(entry);
> +      _mesa_set_remove(def_state->if_uses, entry);
> +   }
>
> -         if (entry2 == NULL) {
> -            printf("%p\n", entry->key);
> -         }
> -      }
> +   if (def_state->if_uses->entries != 0) {
> +      printf("extra entries in register uses:\n");
> +      struct set_entry *entry;
> +      set_foreach(def_state->if_uses, entry)
> +         printf("%p\n", entry->key);
>
>        abort();
>     }
> --
> 2.3.6
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list