[Mesa-dev] [PATCH 2/3] glsl: Use hash tables for brw_fs_vector_splitting().

[Timothy Arceri]

On Sat, 2015-09-05 at 02:21 -0700, Kenneth Graunke wrote:
> Cuts compile/link time of the fragment shader in #91857 by 25%
> (21.64 -> 16.28).
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91857
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  .../drivers/dri/i965/brw_fs_vector_splitting.cpp   | 48 ++++++++++++-------
> ---
>  1 file changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_vector_splitting.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_vector_splitting.cpp
> index 96d4f37..ef1ff03 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_vector_splitting.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_vector_splitting.cpp
> @@ -43,6 +43,7 @@
>  #include "glsl/ir_visitor.h"
>  #include "glsl/ir_rvalue_visitor.h"
>  #include "glsl/glsl_types.h"
> +#include "util/hash_table.h"
>  
>  static bool debug = false;
>  
> @@ -72,11 +73,13 @@ public:
>     ir_vector_reference_visitor(void)
>     {
>        this->mem_ctx = ralloc_context(NULL);
> -      this->variable_list.make_empty();
> +      this->ht = _mesa_hash_table_create(mem_ctx, _mesa_hash_pointer,
> +                                         _mesa_key_pointer_equal);
>     }
>  
>     ~ir_vector_reference_visitor(void)
>     {
> +      _mesa_hash_table_destroy(ht, NULL);

Not a big deal but you created the table with mem_ctx so you dont need to do
this right?

>        ralloc_free(mem_ctx);
>     }
>  
> @@ -89,7 +92,7 @@ public:
>     variable_entry *get_variable_entry(ir_variable *var);
>  
>     /* List of variable_entry */
> -   exec_list variable_list;
> +   struct hash_table *ht;
>  
>     void *mem_ctx;
>  };
> @@ -119,13 +122,12 @@
> ir_vector_reference_visitor::get_variable_entry(ir_variable *var)
>        break;
>     }
>  
> -   foreach_in_list(variable_entry, entry, &variable_list) {
> -      if (entry->var == var)
> -	 return entry;
> -   }
> +   struct hash_entry *hte = _mesa_hash_table_search(ht, var);
> +   if (hte)
> +      return (struct variable_entry *) hte->data;
>  
>     variable_entry *entry = new(mem_ctx) variable_entry(var);
> -   this->variable_list.push_tail(entry);
> +   _mesa_hash_table_insert(ht, var, entry);
>     return entry;
>  }
>  
> @@ -195,9 +197,9 @@
> ir_vector_reference_visitor::visit_enter(ir_function_signature *ir)
>  
>  class ir_vector_splitting_visitor : public ir_rvalue_visitor {
>  public:
> -   ir_vector_splitting_visitor(exec_list *vars)
> +   ir_vector_splitting_visitor(struct hash_table *vars)
>     {
> -      this->variable_list = vars;
> +      this->ht = vars;
>     }
>  
>     virtual ir_visitor_status visit_leave(ir_assignment *);
> @@ -205,7 +207,7 @@ public:
>     void handle_rvalue(ir_rvalue **rvalue);
>     variable_entry *get_splitting_entry(ir_variable *var);
>  
> -   exec_list *variable_list;
> +   struct hash_table *ht;
>  };
>  
>  variable_entry *
> @@ -216,13 +218,8 @@
> ir_vector_splitting_visitor::get_splitting_entry(ir_variable *var)
>     if (!var->type->is_vector())
>        return NULL;
>  
> -   foreach_in_list(variable_entry, entry, variable_list) {
> -      if (entry->var == var) {
> -	 return entry;
> -      }
> -   }
> -
> -   return NULL;
> +   struct hash_entry *hte = _mesa_hash_table_search(ht, var);
> +   return hte ? (struct variable_entry *) hte->data : NULL;
>  }
>  
>  void
> @@ -329,12 +326,16 @@ ir_vector_splitting_visitor::visit_leave(ir_assignment
> *ir)
>  bool
>  brw_do_vector_splitting(exec_list *instructions)
>  {
> +   struct hash_entry *hte;
> +
>     ir_vector_reference_visitor refs;
>  
>     visit_list_elements(&refs, instructions);
>  
>     /* Trim out variables we can't split. */
> -   foreach_in_list_safe(variable_entry, entry, &refs.variable_list) {
> +   bool ht_empty = true;
> +   hash_table_foreach(refs.ht, hte) {
> +      struct variable_entry *entry = (struct variable_entry *) hte->data;
>        if (debug) {
>  	 fprintf(stderr, "vector %s@%p: whole_access %d\n",
>                   entry->var->name, (void *) entry->var,
> @@ -342,11 +343,13 @@ brw_do_vector_splitting(exec_list *instructions)
>        }
>  
>        if (entry->whole_vector_access) {
> -	 entry->remove();
> +         _mesa_hash_table_remove(refs.ht, hte);
> +      } else {
> +         ht_empty = false;
>        }
>     }
>  
> -   if (refs.variable_list.is_empty())
> +   if (ht_empty)

hash_table has an entries field I think you could check if its empty like
this:

if (refs.ht->entries == 0)

Other than this and the comment higher up.

Reviewed-by: Timothy Arceri <t_arceri at yahoo.com.au>


>        return false;
>  
>     void *mem_ctx = ralloc_context(NULL);
> @@ -354,7 +357,8 @@ brw_do_vector_splitting(exec_list *instructions)
>     /* Replace the decls of the vectors to be split with their split
>      * components.
>      */
> -   foreach_in_list(variable_entry, entry, &refs.variable_list) {
> +   hash_table_foreach(refs.ht, hte) {
> +      struct variable_entry *entry = (struct variable_entry *) hte->data;
>        const struct glsl_type *type;
>        type = glsl_type::get_instance(entry->var->type->base_type, 1, 1);
>  
> @@ -378,7 +382,7 @@ brw_do_vector_splitting(exec_list *instructions)
>        entry->var->remove();
>     }
>  
> -   ir_vector_splitting_visitor split(&refs.variable_list);
> +   ir_vector_splitting_visitor split(refs.ht);
>     visit_list_elements(&split, instructions);
>  
>     ralloc_free(mem_ctx);