[Mesa-dev] [PATCH 1/4] nir: Use a list instead of a hash_table for inputs, outputs, and uniforms
Connor Abbott
cwabbott0 at gmail.com
Wed Mar 18 17:22:55 PDT 2015
Reviewed-by: Connor Abbott <cwabbott0 at gmail.com>
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 <jason at jlekstrand.net> 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
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list