<div dir="ltr"><div>Hi all,</div><div> </div><div>Sorry wrong mailing list.</div><div>Just ignore this message.</div><div> </div><div>Regards, </div><div>Andrii. </div><div><div class="gmail_quote"><div dir="ltr">On Fri, Aug 31, 2018 at 5:08 PM <<a href="mailto:asimiklit.work@gmail.com">asimiklit.work@gmail.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Andrii Simiklit <<a href="mailto:andrii.simiklit@globallogic.com" target="_blank">andrii.simiklit@globallogic.com</a>> It fixes a bit incorrectly implemented ARB_program_interface_query. If input aoa element is unused in shader program the 'glGetProgramResourceIndex' function shouldn't return a valid resource index for it according to: ARB_program_interface_query spec: " For an active variable declared as an array of an aggregate data type (structures or arrays), a separate entry will be generated for each active array element" Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=92822" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=92822</a> Signed-off-by: Andrii Simiklit <<a href="mailto:andrii.simiklit@globallogic.com" target="_blank">andrii.simiklit@globallogic.com</a>> --- src/compiler/Makefile.sources | 4 +- .../glsl/ir_collect_active_aoa_elements.cpp | 148 +++++++++++++++++++++ src/compiler/glsl/ir_collect_active_aoa_elements.h | 49 +++++++ src/compiler/glsl/linker.cpp | 75 +++++++++-- src/compiler/glsl/meson.build | 2 + 5 files changed, 265 insertions(+), 13 deletions(-) create mode 100644 src/compiler/glsl/ir_collect_active_aoa_elements.cpp create mode 100644 src/compiler/glsl/ir_collect_active_aoa_elements.h diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources index d3b0656..a2ba3e3 100644 --- a/src/compiler/Makefile.sources +++ b/src/compiler/Makefile.sources @@ -154,7 +154,9 @@ LIBGLSL_FILES = \ glsl/serialize.cpp \ glsl/serialize.h \ glsl/string_to_uint_map.cpp \ - glsl/string_to_uint_map.h + glsl/string_to_uint_map.h \ + glsl/ir_collect_active_aoa_elements.cpp \ + glsl/ir_collect_active_aoa_elements.h LIBGLSL_SHADER_CACHE_FILES = \ glsl/shader_cache.cpp \ diff --git a/src/compiler/glsl/ir_collect_active_aoa_elements.cpp b/src/compiler/glsl/ir_collect_active_aoa_elements.cpp new file mode 100644 index 0000000..50042c7 --- /dev/null +++ b/src/compiler/glsl/ir_collect_active_aoa_elements.cpp @@ -0,0 +1,148 @@ +/* + * Copyright © 2018 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +#include "ir_collect_active_aoa_elements.h" +#include "program.h" +#include "linker_util.h" +#include "util/set.h" +#include "util/u_dynarray.h" + +namespace +{ + /*** + * Helps to collect the names of used aoa elements + * to the accessed_elements set + ***/ + struct elem_inserter + { + elem_inserter(struct set *accessed_elements_) + : accessed_elements(accessed_elements_) + {} + void operator ()(const char *name) + { + if (NULL == _mesa_set_search(accessed_elements, name)) { + _mesa_set_add(accessed_elements, name); + } + } + struct set *accessed_elements; + }; + + bool + ir_is_array_deref(ir_rvalue *const ir) + { + return (ir_type_dereference_array == ir->ir_type); + } + + ir_variable * + base_ir_dereference_var(ir_dereference_array *const ir) + { + ir_dereference_array const *base_ir = ir; + while (ir_type_dereference_array == base_ir->array->ir_type) { + base_ir = base_ir->array->as_dereference_array(); + assert(NULL != base_ir); + } + + ir_dereference_variable *const d = + base_ir->array->as_dereference_variable(); + return (NULL == d) ? NULL : d->var; + } +} +/** + * Helps to produce all combinations of used aoa elements + * for cases with constant and variable index. + **/ +template <typename FunctorType> +inline void enumiramte_active_elements(void *memctx, + ir_dereference_array **first, + ir_dereference_array **item, + const char *accessed_elem_name, + FunctorType functor) +{ + if(item == (first - 1)) { + functor(accessed_elem_name); + return; + } + + ir_dereference_array *const deref = (*item); + ir_constant const *const idx = deref->array_index->as_constant(); + if (NULL != idx) { + const unsigned uidx = idx->get_uint_component(0U); + char *elem = ralloc_asprintf(memctx, "%s[%u]", + accessed_elem_name, uidx); + enumiramte_active_elements(memctx, first, item - 1, elem, functor); + } + else { + for (unsigned i = 0U; i < deref->type->length; ++i) { + char *elem = ralloc_asprintf(memctx, "%s[%u]", + accessed_elem_name, i); + enumiramte_active_elements(memctx, first, item - 1, elem, functor); + } + } +} + +ir_visitor_status +ir_collect_active_aoa_elements::visit_enter(ir_dereference_array *ir) +{ + if (!ir_is_array_deref(ir->array)) + return visit_continue; + + last_array_deref = ir; + if (last_array_deref && last_array_deref->array == ir) + return visit_continue; + + ir_variable *const var = base_ir_dereference_var(ir); + if (!var) + return visit_continue; + + struct util_dynarray indexes; + util_dynarray_init(&indexes, NULL); + /* The first element it is dereference + * of regular array not aoa so skip it. + * All the dereference which are stored in + * the IR tree are storead as in the following instance: + * + * gl_Position = myarr[5][3][1]; + * + * The first dereference will have an array_index = [1] + * The second dereference will have an array_index = [3] + * The third dereference will have an array_index = [5] + * So we need to traverse over the elements in the reverse direction + * to enumerate all combinations of used aoa elements. + */ + ir_rvalue *it = ir->array; + while (ir_is_array_deref(it)) { + ir_dereference_array *deref = it->as_dereference_array(); + assert(NULL != deref); + assert(deref->array->type->is_array()); + util_dynarray_append(&indexes, ir_dereference_array*, deref); + it = deref->array; + } + ir_dereference_array **last = util_dynarray_top_ptr(&indexes, + ir_dereference_array*); + ir_dereference_array **first = util_dynarray_element(&indexes, + ir_dereference_array*, 0); + enumiramte_active_elements(memctx, first, last, var->name, + elem_inserter(accessed_elements)); + util_dynarray_fini(&indexes); + return visit_continue_with_parent; +} \ No newline at end of file diff --git a/src/compiler/glsl/ir_collect_active_aoa_elements.h b/src/compiler/glsl/ir_collect_active_aoa_elements.h new file mode 100644 index 0000000..fb8a8cd --- /dev/null +++ b/src/compiler/glsl/ir_collect_active_aoa_elements.h @@ -0,0 +1,49 @@ +/* + * Copyright © 2018 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +#ifndef IR_COLLECT_ACTIVE_AOA_ELEMENTS_H +#define IR_COLLECT_ACTIVE_AOA_ELEMENTS_H + +#include "ir.h" +#include "util/hash_table.h" + +class ir_collect_active_aoa_elements : public ir_hierarchical_visitor { +public: + ir_collect_active_aoa_elements(void *memctx_, + struct set *accessed_elements_) + : memctx(memctx_), + accessed_elements(accessed_elements_), + last_array_deref(0) + { + } + virtual ir_visitor_status visit_enter(ir_dereference_array *); +private: + void *memctx; + struct set *accessed_elements; + /** + * Used to prevent some redundant calculations. + */ + ir_dereference_array *last_array_deref; +}; + +#endif /* IR_COLLECT_ACTIVE_AOA_ELEMENTS_H */ diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index f08971d..8159404 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -84,6 +84,7 @@ #include "builtin_functions.h" #include "shader_cache.h" #include "util/u_string.h" +#include "ir_collect_active_aoa_elements.h" #include "main/imports.h" #include "main/shaderobj.h" @@ -93,6 +94,24 @@ namespace { +struct set * find_active_aoa_elements(void *mem_ctx, + gl_linked_shader * linked_shader) +{ + struct set *active_aoa_elems = + _mesa_set_create(mem_ctx, _mesa_key_hash_string, + _mesa_key_string_equal); + ir_collect_active_aoa_elements v(mem_ctx, active_aoa_elems); + visit_list_elements(&v, linked_shader->ir); +#if 0 + set_entry *entry; + /* Print the active array of arrays "aoa" elements */ + set_foreach(active_aoa_elems, entry) + { + fprintf(stderr, "used aoa element : %s\n", (const char *)entry->key); + } +#endif + return active_aoa_elems; +} struct find_variable { const char *name; bool found; @@ -3819,7 +3838,8 @@ add_shader_variable(const struct gl_context *ctx, const char *name, const glsl_type *type, bool use_implicit_location, int location, bool inouts_share_location, - const glsl_type *outermost_struct_type = NULL) + const glsl_type *outermost_struct_type = NULL, + struct set *active_aoa_elems = NULL) { const glsl_type *interface_type = var->get_interface_type(); @@ -3881,7 +3901,8 @@ add_shader_variable(const struct gl_context *ctx, stage_mask, programInterface, var, field_name, field->type, use_implicit_location, field_location, - false, outermost_struct_type)) + false, outermost_struct_type, + NULL)) return false; field_location += field->type->count_attribute_slots(false); @@ -3908,17 +3929,36 @@ add_shader_variable(const struct gl_context *ctx, const struct glsl_type *array_type = type->fields.array; if (array_type->base_type == GLSL_TYPE_STRUCT || array_type->base_type == GLSL_TYPE_ARRAY) { + const bool is_aoa = (array_type->fields.array->base_type == + GLSL_TYPE_ARRAY); unsigned elem_location = location; unsigned stride = inouts_share_location ? 0 : array_type->count_attribute_slots(false); for (unsigned i = 0; i < type->length; i++) { char *elem = ralloc_asprintf(shProg, "%s[%d]", name, i); - if (!add_shader_variable(ctx, shProg, resource_set, - stage_mask, programInterface, - var, elem, array_type, - use_implicit_location, elem_location, - false, outermost_struct_type)) - return false; + /*if next type has an array type + * the aoa elem name is not completed yet + * and we will just come here again + **/ + const bool acceptable = is_aoa || + /* if active_aoa_elems is NULL it will work as was */ + !active_aoa_elems || + /* if this aoa element is used + * it can be added as resource + * otherwise shouldn't be added + **/ + _mesa_set_search(active_aoa_elems, elem) != NULL; + + if(acceptable) + { + if (!add_shader_variable(ctx, shProg, resource_set, + stage_mask, programInterface, + var, elem, array_type, + use_implicit_location, elem_location, + false, outermost_struct_type, + active_aoa_elems)) + return false; + } elem_location += stride; } return true; @@ -3964,7 +4004,8 @@ static bool add_interface_variables(const struct gl_context *ctx, struct gl_shader_program *shProg, struct set *resource_set, - unsigned stage, GLenum programInterface) + unsigned stage, GLenum programInterface, + struct set *active_aoa_elems) { exec_list *ir = shProg->_LinkedShaders[stage]->ir; @@ -4017,7 +4058,9 @@ add_interface_variables(const struct gl_context *ctx, 1 << stage, programInterface, var, var->name, var->type, vs_input_or_fs_output, var->data.location - loc_bias, - inout_has_same_location(var, stage))) + inout_has_same_location(var, stage), + NULL, + active_aoa_elems)) return false; } return true; @@ -4354,15 +4397,22 @@ build_program_resource_list(struct gl_context *ctx, if (!add_fragdata_arrays(ctx, shProg, resource_set)) return; + // temporary mem context for program res building + void *res_build_mem_ctx = ralloc_context(NULL); + struct set *active_aoa_elems = find_active_aoa_elements(res_build_mem_ctx, + shProg->_LinkedShaders[input_stage]); /* Add inputs and outputs to the resource list. */ if (!add_interface_variables(ctx, shProg, resource_set, - input_stage, GL_PROGRAM_INPUT)) + input_stage, GL_PROGRAM_INPUT, + active_aoa_elems)) return; if (!add_interface_variables(ctx, shProg, resource_set, - output_stage, GL_PROGRAM_OUTPUT)) + output_stage, GL_PROGRAM_OUTPUT, + active_aoa_elems)) return; + if (shProg->last_vert_prog) { struct gl_transform_feedback_info *linked_xfb = shProg->last_vert_prog->sh.LinkedTransformFeedback; @@ -4477,6 +4527,7 @@ build_program_resource_list(struct gl_context *ctx, } _mesa_set_destroy(resource_set, NULL); + ralloc_free(res_build_mem_ctx); } /** diff --git a/src/compiler/glsl/meson.build b/src/compiler/glsl/meson.build index 7e4dd29..fffae24 100644 --- a/src/compiler/glsl/meson.build +++ b/src/compiler/glsl/meson.build @@ -198,6 +198,8 @@ files_libglsl = files( 'serialize.h', 'shader_cache.cpp', 'shader_cache.h', + 'ir_collect_active_aoa_elements.cpp', + 'ir_collect_active_aoa_elements.h', ) files_libglsl_standalone = files( -- 2.7.4 </blockquote></div></div></div>