[Mesa-dev] [PATCH 4/4] st_glsl_to_tgsi: replace variables tracking list with a hash table

[Samuel Pitoiset]

On 05/30/2017 07:52 AM, Dave Airlie wrote:
> From: Dave Airlie <airlied at redhat.com>
> 
> This removes the linear search which is fail when number of variables
> goes up to 30000 or so.
> ---
>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 44 +++++++++++++++++++++---------
>   1 file changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 0e59aca..87c4b10 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -56,6 +56,7 @@
>   #include "st_nir.h"
>   #include "st_shader_cache.h"
>   
> +#include "util/hash_table.h"
>   #include <algorithm>
>   
>   #define PROGRAM_ANY_CONST ((1 << PROGRAM_STATE_VAR) |    \
> @@ -310,7 +311,9 @@ public:
>      const struct tgsi_opcode_info *info;
>   };
>   
> -class variable_storage : public exec_node {
> +class variable_storage {
> +   DECLARE_RZALLOC_CXX_OPERATORS(variable_storage)
> +
>   public:
>      variable_storage(ir_variable *var, gl_register_file file, int index,
>                       unsigned array_id = 0)
> @@ -488,7 +491,7 @@ public:
>      st_src_reg result;
>   
>      /** List of variable_storage */
> -   exec_list variables;
> +   struct hash_table *variables;
>   
>      /** List of immediate_storage */
>      exec_list immediates;
> @@ -1306,13 +1309,15 @@ glsl_to_tgsi_visitor::get_temp(const glsl_type *type)
>   variable_storage *
>   glsl_to_tgsi_visitor::find_variable_storage(ir_variable *var)
>   {
> +   struct hash_entry *entry = _mesa_hash_table_search(this->variables,
> +                                                      var);
> +   variable_storage *storage;
> +   if (!entry)
> +      return NULL;
>   
> -   foreach_in_list(variable_storage, entry, &this->variables) {
> -      if (entry->var == var)
> -         return entry;
> -   }
> +   storage = (variable_storage *)entry->data;
>   
> -   return NULL;
> +   return storage;
>   }

I would suggest to write to:

{
   struct hash_entry *entry;

   entry = _mesa_hash_table_search(this->variables, var);
   if (!entry)
     return NULL;
   return (variable_storage *)entry->data;
}

I'm just wondering if this might affect very small shaders. I think a 
full shaderdb run can answer the question. :)

>   
>   void
> @@ -1345,7 +1350,8 @@ glsl_to_tgsi_visitor::visit(ir_variable *ir)
>         if (i == ir->get_num_state_slots()) {
>            /* We'll set the index later. */
>            storage = new(mem_ctx) variable_storage(ir, PROGRAM_STATE_VAR, -1);
> -         this->variables.push_tail(storage);
> +
> +         _mesa_hash_table_insert(this->variables, ir, storage);
>   
>            dst = undef_dst;
>         } else {
> @@ -1360,7 +1366,7 @@ glsl_to_tgsi_visitor::visit(ir_variable *ir)
>            storage = new(mem_ctx) variable_storage(ir, dst.file, dst.index,
>                                                    dst.array_id);
>   
> -         this->variables.push_tail(storage);
> +         _mesa_hash_table_insert(this->variables, ir, storage);
>         }
>   
>   
> @@ -2595,7 +2601,7 @@ glsl_to_tgsi_visitor::visit(ir_dereference_variable *ir)
>         case ir_var_uniform:
>            entry = new(mem_ctx) variable_storage(var, PROGRAM_UNIFORM,
>                                                  var->data.param_index);
> -         this->variables.push_tail(entry);
> +         _mesa_hash_table_insert(this->variables, var, entry);
>            break;
>         case ir_var_shader_in: {
>            /* The linker assigns locations for varyings and attributes,
> @@ -2642,7 +2648,7 @@ glsl_to_tgsi_visitor::visit(ir_dereference_variable *ir)
>                                                  decl->array_id);
>            entry->component = component;
>   
> -         this->variables.push_tail(entry);
> +         _mesa_hash_table_insert(this->variables, var, entry);
>            break;
>         }
>         case ir_var_shader_out: {
> @@ -2700,7 +2706,8 @@ glsl_to_tgsi_visitor::visit(ir_dereference_variable *ir)
>            }
>            entry->component = component;
>   
> -         this->variables.push_tail(entry);
> +         _mesa_hash_table_insert(this->variables, var, entry);
> +
>            break;
>         }
>         case ir_var_system_value:
> @@ -2714,7 +2721,7 @@ glsl_to_tgsi_visitor::visit(ir_dereference_variable *ir)
>   
>            entry = new(mem_ctx) variable_storage(var, src.file, src.index,
>                                                  src.array_id);
> -         this->variables.push_tail(entry);
> +         _mesa_hash_table_insert(this->variables, var, entry);
>   
>            break;
>         }
> @@ -4569,10 +4576,19 @@ glsl_to_tgsi_visitor::glsl_to_tgsi_visitor()
>      have_fma = false;
>      use_shared_memory = false;
>      has_tex_txf_lz = false;
> +   variables = NULL;
> +}
> +
> +static void var_destroy(struct hash_entry *entry)
> +{
> +   variable_storage *storage = (variable_storage *)entry->data;
> +
> +   delete storage;
>   }
>   
>   glsl_to_tgsi_visitor::~glsl_to_tgsi_visitor()
>   {
> +   _mesa_hash_table_destroy(variables, var_destroy);
>      free(array_sizes);
>      ralloc_free(mem_ctx);
>   }
> @@ -6772,6 +6788,8 @@ get_mesa_program_tgsi(struct gl_context *ctx,
>      v->has_tex_txf_lz = pscreen->get_param(pscreen,
>                                             PIPE_CAP_TGSI_TEX_TXF_LZ);
>   
> +   v->variables = _mesa_hash_table_create(v->mem_ctx, _mesa_hash_pointer,
> +                                          _mesa_key_pointer_equal);
>      _mesa_generate_parameters_list_for_uniforms(shader_program, shader,
>                                                  prog->Parameters);
>   
>