i965/glsl: don't add unused aoa elements to the program resource list

Submitted by andrey simiklit on Aug. 31, 2018, 2:12 p.m.

Details

Message ID 1535724776-27832-1-git-send-email-asimiklit.work@gmail.com
State New
Headers show
Series "i965/glsl: don't add unused aoa elements to the program resource list" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

andrey simiklit Aug. 31, 2018, 2:12 p.m.
From: Andrii Simiklit <andrii.simiklit@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"

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92822
Signed-off-by: Andrii Simiklit <andrii.simiklit@globallogic.com>
---
 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

Patch hide | download patch | download mbox

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(

Comments

Hi all,

Could somebody run it on CI to confirm that this patch fixes one test and
not add regressions or maybe even take a look at this patch)

Regards,
Andrii.

On Fri, Aug 31, 2018 at 5:13 PM <asimiklit.work@gmail.com> wrote:

> From: Andrii Simiklit <andrii.simiklit@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"
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92822
> Signed-off-by: Andrii Simiklit <andrii.simiklit@globallogic.com>
> ---
>  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
>
>
On 09/07/2018 03:25 PM, andrey simiklit wrote:
> Hi all,
> 
> Could somebody run it on CI to confirm that this patch fixes one test 
> and not add regressions or maybe even take a look at this patch)

CI reports failures in following conformance test:

KHR-GL46.enhanced_layouts.varying_structure_locations

As example:

--- 8< ---
Test case (float) failed.
FAILURE. Test case: mat2. Inspection of separable draw program interface 
failed:
Failed to query program for varying: single. Reason: 
GetProgramResourceiv: glGetError() returned GL_INVALID_VALUE at 
gl4cEnhancedLayoutsTests.cpp:3073
Failed to query program for varying: array. Reason: 
GetProgramResourceiv: glGetError() returned GL_INVALID_VALUE at 
gl4cEnhancedLayoutsTests.cpp:3073
--- 8< ---

There are similar/same failures for each glsl type.

Please find couple of small review notes below.

> Regards,
> Andrii.
> 
> On Fri, Aug 31, 2018 at 5:13 PM <asimiklit.work@gmail.com 
> <mailto:asimiklit.work@gmail.com>> wrote:
> 
>     From: Andrii Simiklit <andrii.simiklit@globallogic.com
>     <mailto:andrii.simiklit@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"
> 
>     Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92822
>     Signed-off-by: Andrii Simiklit <andrii.simiklit@globallogic.com
>     <mailto:andrii.simiklit@globallogic.com>>
>     ---
>       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,

enumerate

>     +                                        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);


I think this should rather use "array_type->is_array_of_arrays()".

Maybe this block here is related to the failures (?) I have a feeling 
that we are handling some resources as aoa resources that are not really 
aoa resources.


>                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
>
Hello,

Thanks a lot for reviewing and testing on CI.
I will send new patch as soon as fix all these issues.

Regards,
Andrii.

On Thu, Sep 13, 2018 at 2:32 PM Tapani Pälli <tapani.palli@intel.com> wrote:

>
>
> On 09/07/2018 03:25 PM, andrey simiklit wrote:
> > Hi all,
> >
> > Could somebody run it on CI to confirm that this patch fixes one test
> > and not add regressions or maybe even take a look at this patch)
>
> CI reports failures in following conformance test:
>
> KHR-GL46.enhanced_layouts.varying_structure_locations
>
> As example:
>
> --- 8< ---
> Test case (float) failed.
> FAILURE. Test case: mat2. Inspection of separable draw program interface
> failed:
> Failed to query program for varying: single. Reason:
> GetProgramResourceiv: glGetError() returned GL_INVALID_VALUE at
> gl4cEnhancedLayoutsTests.cpp:3073
> Failed to query program for varying: array. Reason:
> GetProgramResourceiv: glGetError() returned GL_INVALID_VALUE at
> gl4cEnhancedLayoutsTests.cpp:3073
> --- 8< ---
>
> There are similar/same failures for each glsl type.
>
> Please find couple of small review notes below.
>
> > Regards,
> > Andrii.
> >
> > On Fri, Aug 31, 2018 at 5:13 PM <asimiklit.work@gmail.com
> > <mailto:asimiklit.work@gmail.com>> wrote:
> >
> >     From: Andrii Simiklit <andrii.simiklit@globallogic.com
> >     <mailto:andrii.simiklit@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"
> >
> >     Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92822
> >     Signed-off-by: Andrii Simiklit <andrii.simiklit@globallogic.com
> >     <mailto:andrii.simiklit@globallogic.com>>
> >     ---
> >       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,
>
> enumerate
>
> >     +                                        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);
>
>
> I think this should rather use "array_type->is_array_of_arrays()".
>
> Maybe this block here is related to the failures (?) I have a feeling
> that we are handling some resources as aoa resources that are not really
> aoa resources.
>
>
> >                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
> >
>