[Mesa-dev] [PATCH v2] i965/glsl: don't add unused aoa element to the program resource list
asimiklit.work at gmail.com
asimiklit.work at gmail.com
Thu Sep 20 14:03:06 UTC 2018
From: Andrii Simiklit <andrii.simiklit at globallogic.com>
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"
v2: - Fixed case where the non-AoA elements were removed form
the program resource list.
The KHR-GL46.enhanced_layouts.varying_structure_locations
test is passed now
- Fixed case where a dereference index is not constant:
The proper array length stored in 'deref->array->type->length'
not in 'deref->type->length'
- Fixed the output AoA elements handling:
The list of active input AoA elements should not be used for
output AoA elements
- Fixed the function name from 'enumiramte_active_elements' to
'enumerate_active_elements'
- Fixed errors handling during active AoA elements enumeration
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92822
Signed-off-by: Andrii Simiklit <andrii.simiklit at globallogic.com>
---
src/compiler/Makefile.sources | 4 +-
.../glsl/ir_collect_active_aoa_elements.cpp | 155 ++++++++++++++++++
.../glsl/ir_collect_active_aoa_elements.h | 52 ++++++
src/compiler/glsl/linker.cpp | 91 ++++++++--
src/compiler/glsl/meson.build | 2 +
5 files changed, 289 insertions(+), 15 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 d3b0656483..a2ba3e37f1 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 0000000000..df0a41ad3e
--- /dev/null
+++ b/src/compiler/glsl/ir_collect_active_aoa_elements.cpp
@@ -0,0 +1,155 @@
+/*
+ * 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_)
+ {}
+ bool operator ()(const char *name)
+ {
+ bool oval = true;
+ if (NULL == _mesa_set_search(accessed_elements, name)) {
+ oval = (_mesa_set_add(accessed_elements, name) != NULL);
+ }
+ return oval;
+ }
+ 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 bool enumerate_active_elements(void *memctx,
+ ir_dereference_array **first,
+ ir_dereference_array **item,
+ const char *accessed_elem_name,
+ FunctorType functor)
+{
+ if (NULL == accessed_elem_name) {
+ return false;
+ }
+ if(item == (first - 1))
+ return functor(accessed_elem_name);
+
+ 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);
+
+ if(!enumerate_active_elements(memctx, first, item - 1, elem, functor))
+ return false;
+ }
+ else {
+ for (unsigned i = 0U; i < deref->array->type->length; ++i) {
+ char *elem = ralloc_asprintf(memctx, "%s[%u]",
+ accessed_elem_name, i);
+ if(!enumerate_active_elements(memctx, first, item - 1, elem, functor))
+ return false;
+ }
+ }
+ return true;
+}
+
+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);
+ error = !enumerate_active_elements(memctx, first, last, var->name,
+ elem_inserter(accessed_elements));
+ util_dynarray_fini(&indexes);
+ return error ? visit_stop : 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 0000000000..5ca1dca7ac
--- /dev/null
+++ b/src/compiler/glsl/ir_collect_active_aoa_elements.h
@@ -0,0 +1,52 @@
+/*
+ * 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),
+ error(false)
+ {
+ }
+ virtual ir_visitor_status visit_enter(ir_dereference_array *);
+ bool has_error() const { return this->error; }
+private:
+ void *memctx;
+ struct set *accessed_elements;
+ /**
+ * Used to prevent some redundant calculations.
+ */
+ ir_dereference_array *last_array_deref;
+ bool error;
+};
+
+#endif /* IR_COLLECT_ACTIVE_AOA_ELEMENTS_H */
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 3fde7e78d3..b41cd735ae 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -83,6 +83,7 @@
#include "ir_uniform.h"
#include "builtin_functions.h"
#include "shader_cache.h"
+#include "ir_collect_active_aoa_elements.h"
#include "util/u_string.h"
#include "util/u_math.h"
@@ -94,6 +95,32 @@
namespace {
+struct set * find_active_aoa_elements(void *mem_ctx,
+ gl_linked_shader * linked_shader,
+ struct gl_shader_program *shProg)
+{
+ 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
+ if(!v.has_error()) {
+ 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);
+ }
+ } else {
+ fprintf(stderr, "error: ir_collect_active_aoa_elements completed with an error!\n");
+ }
+#endif
+ if(v.has_error()) {
+ linker_error(shProg, "error during active AoA elements enumeration!\n");
+ }
+ return v.has_error() ? NULL : active_aoa_elems;
+}
struct find_variable {
const char *name;
bool found;
@@ -3880,7 +3907,9 @@ 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,
+ uint32_t array_dimension = 0)
{
const glsl_type *interface_type = var->get_interface_type();
@@ -3942,7 +3971,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, 0U))
return false;
field_location += field->type->count_attribute_slots(false);
@@ -3967,19 +3997,40 @@ add_shader_variable(const struct gl_context *ctx,
* separate active variable."
*/
const struct glsl_type *array_type = type->fields.array;
- if (array_type->base_type == GLSL_TYPE_STRUCT ||
- array_type->base_type == GLSL_TYPE_ARRAY) {
+ if (array_type->is_record() || array_type->is_array()) {
+ const bool is_aoa = array_type->is_array_of_arrays();
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;
+ /* Single dimensional array of structs is accepted by default*/
+ const bool acceptable_elem = (array_dimension < 1u &&
+ array_type->is_record()) ||
+ /*if next type has an aoa type
+ * the aoa elem name is not completed yet
+ * and we will just come here again
+ **/
+ is_aoa ||
+ /* if active_aoa_elems is NULL we will add all elements
+ */
+ (NULL == active_aoa_elems) ||
+ /* if this aoa element is used
+ * it can be added as resource
+ * otherwise shouldn't be added
+ **/
+ (NULL != _mesa_set_search(active_aoa_elems, elem));
+ if(acceptable_elem) {
+ if (!add_shader_variable(ctx, shProg, resource_set,
+ stage_mask, programInterface,
+ var, elem, array_type,
+ use_implicit_location, elem_location,
+ false, outermost_struct_type,
+ /*Pass the NULL to accept all elements*/
+ is_aoa ? active_aoa_elems : NULL,
+ array_dimension + 1U))
+ return false;
+ }
elem_location += stride;
}
return true;
@@ -4025,7 +4076,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;
@@ -4078,7 +4130,8 @@ 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;
@@ -4415,13 +4468,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 *in_active_aoa_elems = find_active_aoa_elements(res_build_mem_ctx,
+ shProg->_LinkedShaders[input_stage], shProg);
/* 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,
+ in_active_aoa_elems))
return;
+ struct set *out_active_aoa_elems = find_active_aoa_elements(res_build_mem_ctx,
+ shProg->_LinkedShaders[output_stage], shProg);
+
if (!add_interface_variables(ctx, shProg, resource_set,
- output_stage, GL_PROGRAM_OUTPUT))
+ output_stage, GL_PROGRAM_OUTPUT,
+ out_active_aoa_elems))
return;
if (shProg->last_vert_prog) {
@@ -4538,6 +4600,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 7e4dd2929a..fffae246af 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.17.1
More information about the mesa-dev
mailing list