[Mesa-dev] [PATCH 11/13] nir/nir: Use a linked list instead of a has set for use/def sets

Connor Abbott cwabbott0 at gmail.com
Thu May 7 15:37:19 PDT 2015


Based on the testing you did, it sounds like switching to linked lists
gives us some pretty good performance gains, but before we go ahead
with this you should collect some numbers using
http://anholt.net/compare-perf/ and put them on this commit message.
Comparing list vs. no-list as well as NIR vs. non-NIR might be useful,
so we can compare the time saved to the total time we spend doing
NIR-related things.

On Tue, Apr 28, 2015 at 12:03 AM, 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          | 228 +++++++++++++++-----------------------------
>  src/glsl/nir/nir.h          |  45 +++++++--
>  src/glsl/nir/nir_validate.c | 158 +++++++++++++++---------------
>  3 files changed, 194 insertions(+), 237 deletions(-)
>
> diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
> index b8f5dd4..be13c90 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);
> +   list_inithead(&reg->uses);
> +   list_inithead(&reg->defs);
> +   list_inithead(&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) {
> +      list_addtail(&if_stmt->condition.use_link,
> +                   &if_stmt->condition.ssa->if_uses);
> +   } else {
> +      list_addtail(&if_stmt->condition.use_link,
> +                   &if_stmt->condition.reg.reg->if_uses);
> +   }
>  }
>
>  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);
> +      list_del(&if_stmt->condition.use_link);
>        break;
>     }
>
> @@ -1293,9 +1284,9 @@ 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;
> +   list_addtail(&src->use_link,
> +                src->is_ssa ? &src->ssa->uses : &src->reg.reg->uses);
>
>     return true;
>  }
> @@ -1320,8 +1311,10 @@ 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;
> +      list_addtail(&dest->reg.def_link, &dest->reg.reg->defs);
> +   }
>
>     return true;
>  }
> @@ -1436,13 +1429,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);
> +   list_del(&src->use_link);
>
>     return true;
>  }
> @@ -1450,16 +1437,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)
> +      list_del(&dest->reg.def_link);
>
>     return true;
>  }
> @@ -1834,86 +1813,65 @@ 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;
> +      list_del(&src->use_link);
>     }
>  }
>
> -void
> -nir_instr_rewrite_src(nir_instr *instr, nir_src *src, nir_src new_src)
> -{
> -   nir_src old_src = *src;
> -   *src = new_src;
> -
> -   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);
> -         }
> +static void
> +src_add_all_uses(nir_src *src, nir_instr *parent_instr, nir_if *parent_if)
> +{
> +   for (; src; src = src->is_ssa ? NULL : src->reg.indirect) {
> +      if (!src_is_valid(src))
> +         continue;
> +
> +      if (parent_instr) {
> +         src->parent_instr = parent_instr;
> +         if (src->is_ssa)
> +            list_addtail(&src->use_link, &src->ssa->uses);
> +         else
> +            list_addtail(&src->use_link, &src->reg.reg->uses);
>        } 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)
> +            list_addtail(&src->use_link, &src->ssa->if_uses);
> +         else
> +            list_addtail(&src->use_link, &src->reg.reg->if_uses);
>        }
>     }
> +}
>
> -   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 +1880,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);
> +   list_inithead(&def->uses);
> +   list_inithead(&def->if_uses);
>     def->num_components = num_components;
>
>     if (instr->block) {
> @@ -1946,57 +1902,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_foreach_use_safe(def, use_src) {
> +      nir_instr *src_parent_instr = use_src->parent_instr;
> +      list_del(&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_foreach_if_use_safe(def, use_src) {
> +      nir_if *src_parent_if = use_src->parent_if;
> +      list_del(&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..ac027b0 100644
> --- a/src/glsl/nir/nir.h
> +++ b/src/glsl/nir/nir.h
> @@ -30,6 +30,7 @@
>  #include "util/hash_table.h"
>  #include "../list.h"
>  #include "GL/gl.h" /* GLenum */
> +#include "util/list.h"
>  #include "util/ralloc.h"
>  #include "util/set.h"
>  #include "util/bitset.h"
> @@ -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;
> +   struct list_head uses;
>
>     /** set of nir_instr's where this register is defined (written to) */
> -   struct set *defs;
> +   struct list_head defs;
>
>     /** set of nir_if's where this register is used as a condition */
> -   struct set *if_uses;
> +   struct list_head 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;
> +   struct list_head uses;
>
>     /** set of nir_if's where this register is used as a condition */
> -   struct set *if_uses;
> +   struct list_head if_uses;
>
>     uint8_t num_components;
>  } nir_ssa_def;
> @@ -481,6 +482,9 @@ typedef struct {
>  } nir_reg_src;
>
>  typedef struct {
> +   nir_instr *parent_instr;
> +   struct list_head 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;
> +   };
> +
> +   struct list_head use_link;
> +
> +   union {
>        nir_reg_src reg;
>        nir_ssa_def *ssa;
>     };
> @@ -497,7 +510,19 @@ typedef struct nir_src {
>     bool is_ssa;
>  } nir_src;
>
> -#define NIR_SRC_INIT (nir_src) { { { NULL } } }
> +#define NIR_SRC_INIT (nir_src) { { NULL } }
> +
> +#define nir_foreach_use(reg_or_ssa_def, src) \
> +   list_for_each_entry(nir_src, src, &(reg_or_ssa_def)->uses, use_link)
> +
> +#define nir_foreach_use_safe(reg_or_ssa_def, src) \
> +   list_for_each_entry_safe(nir_src, src, &(reg_or_ssa_def)->uses, use_link)
> +
> +#define nir_foreach_if_use(reg_or_ssa_def, src) \
> +   list_for_each_entry(nir_src, src, &(reg_or_ssa_def)->if_uses, use_link)
> +
> +#define nir_foreach_if_use_safe(reg_or_ssa_def, src) \
> +   list_for_each_entry_safe(nir_src, src, &(reg_or_ssa_def)->if_uses, use_link)
>
>  typedef struct {
>     union {
> @@ -510,6 +535,12 @@ typedef struct {
>
>  #define NIR_DEST_INIT (nir_dest) { { { NULL } } }
>
> +#define nir_foreach_def(reg, dest) \
> +   list_for_each_entry(nir_dest, dest, &(reg)->defs, reg.def_link)
> +
> +#define nir_foreach_def_safe(reg, dest) \
> +   list_for_each_entry_safe(nir_dest, dest, &(reg)->defs, reg.def_link)
> +
>  static inline nir_src
>  nir_src_for_ssa(nir_ssa_def *def)
>  {
> @@ -1208,7 +1239,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..da92ed9 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);
>
> +   list_validate(&def->uses);
> +   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);
>
> +   list_validate(&reg->uses);
> +   list_validate(&reg->defs);
> +   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_foreach_use(reg, src) {
> +      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_foreach_if_use(reg, src) {
> +      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_foreach_def(reg, src) {
> +      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_foreach_use(def, src) {
> +      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_foreach_if_use(def, src) {
> +      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