From a5d3e061af9451758479f97df97afd6351e7d117 Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Mon, 15 Jun 2020 14:03:59 +1000 Subject: [PATCH] glsl: fix uniform array resizing in the nir linker MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The initial support tried to match uniform variables from different shaders based on the variables pointer. This will obviously never work, instead here we use the variables name whcih also means we must disable this optimisation for spirv. Using the base variable name works because when collecting uniform references we never iterate past the first array dimension, and only support resizing 1D arrays (we also don't support resizing arrays inside structs). We also drop the resized bool as we can't skip processing the var just because is was resized in another shader, we must resize the var in all shaders. Fixes: a34cc97ca3e1 ("glsl: when NIR linker enable use it to resize uniform arrays") Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/3130 Reviewed-by: Tapani Pälli Part-of: --- src/compiler/glsl/gl_nir_link_uniforms.c | 86 ++++++++++++------------ 1 file changed, 42 insertions(+), 44 deletions(-) diff --git a/src/compiler/glsl/gl_nir_link_uniforms.c b/src/compiler/glsl/gl_nir_link_uniforms.c index b1187c45dc1..78fee5b75b4 100644 --- a/src/compiler/glsl/gl_nir_link_uniforms.c +++ b/src/compiler/glsl/gl_nir_link_uniforms.c @@ -42,9 +42,6 @@ struct uniform_array_info { /** Set of bit-flags to note which array elements have been accessed. */ BITSET_WORD *indices; - - /** Have we rezized this array yet */ - bool resized; }; /** @@ -133,12 +130,9 @@ update_array_sizes(struct gl_shader_program *prog, nir_variable *var, continue; struct hash_entry *entry = - _mesa_hash_table_search(referenced_uniforms[stage], var); + _mesa_hash_table_search(referenced_uniforms[stage], var->name); if (entry) { ainfo = (struct uniform_array_info *) entry->data; - if (ainfo->resized) - return; - max_array_size = MAX2(BITSET_LAST_BIT(ainfo->indices, words), max_array_size); } @@ -172,7 +166,7 @@ update_array_sizes(struct gl_shader_program *prog, nir_variable *var, continue; struct hash_entry *entry = - _mesa_hash_table_search(referenced_uniforms[stage], var); + _mesa_hash_table_search(referenced_uniforms[stage], var->name); if (entry) { struct uniform_array_info *ainfo = (struct uniform_array_info *) entry->data; @@ -181,9 +175,6 @@ update_array_sizes(struct gl_shader_program *prog, nir_variable *var, } } } - - if (ainfo) - ainfo->resized = true; } } @@ -467,7 +458,7 @@ add_var_use_deref(nir_deref_instr *deref, struct hash_table *live, struct uniform_array_info *ainfo = NULL; struct hash_entry *entry = - _mesa_hash_table_search(live, deref->var); + _mesa_hash_table_search(live, deref->var->name); if (!entry && glsl_type_is_array(deref->var->type)) { ainfo = ralloc(live, struct uniform_array_info); @@ -476,8 +467,6 @@ add_var_use_deref(nir_deref_instr *deref, struct hash_table *live, ainfo->deref_list = ralloc(live, struct util_dynarray); util_dynarray_init(ainfo->deref_list, live); - - ainfo->resized = false; } if (entry) @@ -499,7 +488,7 @@ add_var_use_deref(nir_deref_instr *deref, struct hash_table *live, } assert(deref->mode == deref->var->data.mode); - _mesa_hash_table_insert(live, deref->var, ainfo); + _mesa_hash_table_insert(live, deref->var->name, ainfo); } /* Iterate over the shader and collect infomation about uniform use */ @@ -946,11 +935,12 @@ find_and_update_named_uniform_storage(struct gl_context *ctx, update_uniforms_shader_info(prog, state, uniform, type, stage); const struct glsl_type *type_no_array = glsl_without_array(type); - struct hash_entry *entry = + struct hash_entry *entry = prog->data->spirv ? NULL : _mesa_hash_table_search(state->referenced_uniforms[stage], - state->current_var); + state->current_var->name); if (entry != NULL || - glsl_get_base_type(type_no_array) == GLSL_TYPE_SUBROUTINE) + glsl_get_base_type(type_no_array) == GLSL_TYPE_SUBROUTINE || + prog->data->spirv) uniform->active_shader_mask |= 1 << stage; if (!state->var_is_in_block) @@ -1329,11 +1319,12 @@ nir_link_uniform(struct gl_context *ctx, uniform->top_level_array_size = state->top_level_array_size; uniform->top_level_array_stride = state->top_level_array_stride; - struct hash_entry *entry = + struct hash_entry *entry = prog->data->spirv ? NULL : _mesa_hash_table_search(state->referenced_uniforms[stage], - state->current_var); + state->current_var->name); if (entry != NULL || - glsl_get_base_type(type_no_array) == GLSL_TYPE_SUBROUTINE) + glsl_get_base_type(type_no_array) == GLSL_TYPE_SUBROUTINE || + prog->data->spirv) uniform->active_shader_mask |= 1 << stage; if (location >= 0) { @@ -1508,29 +1499,31 @@ gl_nir_link_uniforms(struct gl_context *ctx, /* Iterate through all linked shaders */ struct nir_link_uniforms_state state = {0,}; - /* Gather information on uniform use */ - for (unsigned stage = 0; stage < MESA_SHADER_STAGES; stage++) { - struct gl_linked_shader *sh = prog->_LinkedShaders[stage]; - if (!sh) - continue; + if (!prog->data->spirv) { + /* Gather information on uniform use */ + for (unsigned stage = 0; stage < MESA_SHADER_STAGES; stage++) { + struct gl_linked_shader *sh = prog->_LinkedShaders[stage]; + if (!sh) + continue; - state.referenced_uniforms[stage] = - _mesa_hash_table_create(NULL, _mesa_hash_pointer, - _mesa_key_pointer_equal); + state.referenced_uniforms[stage] = + _mesa_hash_table_create(NULL, _mesa_hash_string, + _mesa_key_string_equal); - nir_shader *nir = sh->Program->nir; - add_var_use_shader(nir, state.referenced_uniforms[stage]); - } + nir_shader *nir = sh->Program->nir; + add_var_use_shader(nir, state.referenced_uniforms[stage]); + } - /* Resize uniform arrays based on the maximum array index */ - for (unsigned stage = 0; stage < MESA_SHADER_STAGES; stage++) { - struct gl_linked_shader *sh = prog->_LinkedShaders[stage]; - if (!sh) - continue; + /* Resize uniform arrays based on the maximum array index */ + for (unsigned stage = 0; stage < MESA_SHADER_STAGES; stage++) { + struct gl_linked_shader *sh = prog->_LinkedShaders[stage]; + if (!sh) + continue; - nir_shader *nir = sh->Program->nir; - nir_foreach_variable(var, &nir->uniforms) - update_array_sizes(prog, var, state.referenced_uniforms); + nir_shader *nir = sh->Program->nir; + nir_foreach_variable(var, &nir->uniforms) + update_array_sizes(prog, var, state.referenced_uniforms); + } } /* Count total number of uniforms and allocate storage */ @@ -1689,7 +1682,8 @@ gl_nir_link_uniforms(struct gl_context *ctx, buffer_block_index = i; struct hash_entry *entry = - _mesa_hash_table_search(state.referenced_uniforms[shader_type], var); + _mesa_hash_table_search(state.referenced_uniforms[shader_type], + var->name); if (entry) { struct uniform_array_info *ainfo = (struct uniform_array_info *) entry->data; @@ -1704,7 +1698,8 @@ gl_nir_link_uniforms(struct gl_context *ctx, buffer_block_index = i; struct hash_entry *entry = - _mesa_hash_table_search(state.referenced_uniforms[shader_type], var); + _mesa_hash_table_search(state.referenced_uniforms[shader_type], + var->name); if (entry) blocks[i].stageref |= 1U << shader_type; @@ -1767,7 +1762,7 @@ gl_nir_link_uniforms(struct gl_context *ctx, location = j; struct hash_entry *entry = - _mesa_hash_table_search(state.referenced_uniforms[shader_type], var); + _mesa_hash_table_search(state.referenced_uniforms[shader_type], var->name); if (entry) blocks[i].stageref |= 1U << shader_type; @@ -1824,7 +1819,10 @@ gl_nir_link_uniforms(struct gl_context *ctx, return false; } - _mesa_hash_table_destroy(state.referenced_uniforms[shader_type], NULL); + if (!prog->data->spirv) { + _mesa_hash_table_destroy(state.referenced_uniforms[shader_type], + NULL); + } if (state.num_shader_samplers > ctx->Const.Program[shader_type].MaxTextureImageUnits) { -- 2.30.2