Reviewed-by: Connor Abbott <[email protected]> A case of premature optimization I guess... I think I did it for linking, but we'll cross that bridge when we come to it.
On Wed, Mar 18, 2015 at 7:45 PM, Jason Ekstrand <[email protected]> wrote: > We never did a single hash table lookup in the entire NIR code base that I > found so there was no real benifit to doing it that way. I suppose that > for linking, we'll probably want to be able to lookup by name but we can > leave building that hash table to the linker. In the mean time this was > causing problems with GLSL IR -> NIR because GLSL IR doesn't guarantee us > unique names of uniforms, etc. This was causing massive rendering isues in > the unreal4 Sun Temple demo. > --- > src/glsl/nir/glsl_to_nir.cpp | 6 +++--- > src/glsl/nir/nir.c | 9 +++------ > src/glsl/nir/nir.h | 6 +++--- > src/glsl/nir/nir_lower_io.c | 13 +++++-------- > src/glsl/nir/nir_print.c | 14 ++++++-------- > src/glsl/nir/nir_validate.c | 16 +++++++++------- > src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 13 +++---------- > 7 files changed, 32 insertions(+), 45 deletions(-) > > diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp > index 047cb51..357944d 100644 > --- a/src/glsl/nir/glsl_to_nir.cpp > +++ b/src/glsl/nir/glsl_to_nir.cpp > @@ -352,15 +352,15 @@ nir_visitor::visit(ir_variable *ir) > break; > > case nir_var_shader_in: > - _mesa_hash_table_insert(shader->inputs, var->name, var); > + exec_list_push_tail(&shader->inputs, &var->node); > break; > > case nir_var_shader_out: > - _mesa_hash_table_insert(shader->outputs, var->name, var); > + exec_list_push_tail(&shader->outputs, &var->node); > break; > > case nir_var_uniform: > - _mesa_hash_table_insert(shader->uniforms, var->name, var); > + exec_list_push_tail(&shader->uniforms, &var->node); > break; > > case nir_var_system_value: > diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c > index abad3f8..6459d51 100644 > --- a/src/glsl/nir/nir.c > +++ b/src/glsl/nir/nir.c > @@ -33,12 +33,9 @@ nir_shader_create(void *mem_ctx, const > nir_shader_compiler_options *options) > { > nir_shader *shader = ralloc(mem_ctx, nir_shader); > > - shader->uniforms = _mesa_hash_table_create(shader, _mesa_key_hash_string, > - _mesa_key_string_equal); > - shader->inputs = _mesa_hash_table_create(shader, _mesa_key_hash_string, > - _mesa_key_string_equal); > - shader->outputs = _mesa_hash_table_create(shader, _mesa_key_hash_string, > - _mesa_key_string_equal); > + exec_list_make_empty(&shader->uniforms); > + exec_list_make_empty(&shader->inputs); > + exec_list_make_empty(&shader->outputs); > > shader->options = options; > > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h > index 669a26e..6b42df9 100644 > --- a/src/glsl/nir/nir.h > +++ b/src/glsl/nir/nir.h > @@ -1380,13 +1380,13 @@ typedef struct nir_shader_compiler_options { > > typedef struct nir_shader { > /** hash table of name -> uniform nir_variable */ > - struct hash_table *uniforms; > + struct exec_list uniforms; > > /** hash table of name -> input nir_variable */ > - struct hash_table *inputs; > + struct exec_list inputs; > > /** hash table of name -> output nir_variable */ > - struct hash_table *outputs; > + struct exec_list outputs; > > /** Set of driver-specific options for the shader. > * > diff --git a/src/glsl/nir/nir_lower_io.c b/src/glsl/nir/nir_lower_io.c > index 207f8da..37c357e 100644 > --- a/src/glsl/nir/nir_lower_io.c > +++ b/src/glsl/nir/nir_lower_io.c > @@ -77,14 +77,11 @@ type_size(const struct glsl_type *type) > } > > static void > -assign_var_locations(struct hash_table *ht, unsigned *size) > +assign_var_locations(struct exec_list *var_list, unsigned *size) > { > unsigned location = 0; > > - struct hash_entry *entry; > - hash_table_foreach(ht, entry) { > - nir_variable *var = (nir_variable *) entry->data; > - > + foreach_list_typed(nir_variable, var, node, var_list) { > /* > * UBO's have their own address spaces, so don't count them towards the > * number of global uniforms > @@ -102,9 +99,9 @@ assign_var_locations(struct hash_table *ht, unsigned *size) > static void > assign_var_locations_shader(nir_shader *shader) > { > - assign_var_locations(shader->inputs, &shader->num_inputs); > - assign_var_locations(shader->outputs, &shader->num_outputs); > - assign_var_locations(shader->uniforms, &shader->num_uniforms); > + assign_var_locations(&shader->inputs, &shader->num_inputs); > + assign_var_locations(&shader->outputs, &shader->num_outputs); > + assign_var_locations(&shader->uniforms, &shader->num_uniforms); > } > > static bool > diff --git a/src/glsl/nir/nir_print.c b/src/glsl/nir/nir_print.c > index f8b14a1..fa11a31 100644 > --- a/src/glsl/nir/nir_print.c > +++ b/src/glsl/nir/nir_print.c > @@ -844,18 +844,16 @@ nir_print_shader(nir_shader *shader, FILE *fp) > print_var_state state; > init_print_state(&state); > > - struct hash_entry *entry; > - > - hash_table_foreach(shader->uniforms, entry) { > - print_var_decl((nir_variable *) entry->data, &state, fp); > + foreach_list_typed(nir_variable, var, node, &shader->uniforms) { > + print_var_decl(var, &state, fp); > } > > - hash_table_foreach(shader->inputs, entry) { > - print_var_decl((nir_variable *) entry->data, &state, fp); > + foreach_list_typed(nir_variable, var, node, &shader->inputs) { > + print_var_decl(var, &state, fp); > } > > - hash_table_foreach(shader->outputs, entry) { > - print_var_decl((nir_variable *) entry->data, &state, fp); > + foreach_list_typed(nir_variable, var, node, &shader->outputs) { > + print_var_decl(var, &state, fp); > } > > foreach_list_typed(nir_variable, var, node, &shader->globals) { > diff --git a/src/glsl/nir/nir_validate.c b/src/glsl/nir/nir_validate.c > index a3fe9d6..f247ae0 100644 > --- a/src/glsl/nir/nir_validate.c > +++ b/src/glsl/nir/nir_validate.c > @@ -931,17 +931,19 @@ nir_validate_shader(nir_shader *shader) > > state.shader = shader; > > - struct hash_entry *entry; > - hash_table_foreach(shader->uniforms, entry) { > - validate_var_decl((nir_variable *) entry->data, true, &state); > + exec_list_validate(&shader->uniforms); > + foreach_list_typed(nir_variable, var, node, &shader->uniforms) { > + validate_var_decl(var, true, &state); > } > > - hash_table_foreach(shader->inputs, entry) { > - validate_var_decl((nir_variable *) entry->data, true, &state); > + exec_list_validate(&shader->inputs); > + foreach_list_typed(nir_variable, var, node, &shader->inputs) { > + validate_var_decl(var, true, &state); > } > > - hash_table_foreach(shader->outputs, entry) { > - validate_var_decl((nir_variable *) entry->data, true, &state); > + exec_list_validate(&shader->outputs); > + foreach_list_typed(nir_variable, var, node, &shader->outputs) { > + validate_var_decl(var, true, &state); > } > > exec_list_validate(&shader->globals); > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > index 69c5680..777914e 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > @@ -205,9 +205,7 @@ fs_visitor::emit_nir_code() > void > fs_visitor::nir_setup_inputs(nir_shader *shader) > { > - struct hash_entry *entry; > - hash_table_foreach(shader->inputs, entry) { > - nir_variable *var = (nir_variable *) entry->data; > + foreach_list_typed(nir_variable, var, node, &shader->inputs) { > enum brw_reg_type type = brw_type_for_base_type(var->type); > fs_reg input = offset(nir_inputs, var->data.driver_location); > > @@ -259,9 +257,7 @@ fs_visitor::nir_setup_outputs(nir_shader *shader) > { > brw_wm_prog_key *key = (brw_wm_prog_key*) this->key; > > - struct hash_entry *entry; > - hash_table_foreach(shader->outputs, entry) { > - nir_variable *var = (nir_variable *) entry->data; > + foreach_list_typed(nir_variable, var, node, &shader->outputs) { > fs_reg reg = offset(nir_outputs, var->data.driver_location); > > int vector_elements = > @@ -313,10 +309,7 @@ fs_visitor::nir_setup_uniforms(nir_shader *shader) > if (dispatch_width != 8) > return; > > - struct hash_entry *entry; > - hash_table_foreach(shader->uniforms, entry) { > - nir_variable *var = (nir_variable *) entry->data; > - > + foreach_list_typed(nir_variable, var, node, &shader->uniforms) { > /* UBO's and atomics don't take up space in the uniform file */ > > if (var->interface_type != NULL || var->type->contains_atomic()) > -- > 2.3.2 > > _______________________________________________ > mesa-dev mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
