[Mesa-dev] [PATCH 4/4] st_glsl_to_tgsi: replace variables tracking list with a hash table
Timothy Arceri
tarceri at itsqueeze.com
Wed Jun 7 10:26:45 UTC 2017
On 01/06/17 23:42, Samuel Pitoiset wrote
> 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;
> }
With this small tidy up suggested by Samuel patches 1, 3 and 4 are:
Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
I've done a shader-db run with those patches, the time slightly improved
but nothing huge, there was also no changes reported. I've also done a
piglit run with the gpu profile and there are no regressions.
Patch 2 however did report a difference in shader-db so I'll try to take
a closer look at that one tomorrow.
>
> 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);
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list