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