[v2] i965/glsl: don't add unused aoa element to the program resource list

Submitted by andrey simiklit on Sept. 20, 2018, 2:03 p.m.

Details

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

Not browsing as part of any series.

Commit Message

andrey simiklit Sept. 20, 2018, 2:03 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"

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@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

Patch hide | download patch | download mbox

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(

Comments

Hi all,

I have tested it on Intel CI and it just fixes test without any regression:
https://mesa-ci.01.org/global_logic/builds/27/group/63a9f0ea7bb98050796b649e85481845

This patch fixes piglit test:
https://bugs.freedesktop.org/show_bug.cgi?id=92822

At the moment it is marked as expected failure.
So it fails with message: "ERROR: This test passed when it expected failure"

It would be grate if somebody reviews this patch)

Thanks,
Andrii.
On Thu, Sep 20, 2018 at 5:03 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"
>
> 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@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
>
>
Sorry for not getting back sooner on this one.

I'm leaning towards a NAK on this one. This is just under 300 new lines 
of code to work around a possibly over strict piglit test. While the 
test is not wrong an implementation is also not required to optimise 
away these unused elements.

If this was actually compressing/optimising the arrays 300+ plus lines 
would probably be ok. But all this code just to hide the elements from 
the resource list seems excessive. I know I suggested this as a possible 
alternative fix to actually optimising the arrays but now that I've seen 
the result I'm not so sure this is a good idea. This is my opinion happy 
to see what others think.

On 21/9/18 12:03 am, 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"
> 
> 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@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(
>
Hi;

On 11/13/18 4:28 AM, Timothy Arceri wrote:
> Sorry for not getting back sooner on this one.
> 
> I'm leaning towards a NAK on this one. This is just under 300 new lines 
> of code to work around a possibly over strict piglit test. While the 
> test is not wrong an implementation is also not required to optimise 
> away these unused elements.
> 
> If this was actually compressing/optimising the arrays 300+ plus lines 
> would probably be ok. But all this code just to hide the elements from 
> the resource list seems excessive. I know I suggested this as a possible 
> alternative fix to actually optimising the arrays but now that I've seen 
> the result I'm not so sure this is a good idea. This is my opinion happy 
> to see what others think.

This is exactly how I feel as well. Functionally correct but way too 
much code considering the issue :/ I believe we do detect elements that 
get used during complication and linking so perhaps do that and in the 
end modify resource list, remove (or mark dirty) all the unused ones?


> On 21/9/18 12:03 am, 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"
>>
>> 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@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(
>>