[Mesa-dev] [PATCH 08/10] glsl/linker: Modify array_sizing_visitor to handle named interface blocks.

[Jordan Justen]

On Fri, Sep 27, 2013 at 12:05 PM, Paul Berry <stereotype441 at gmail.com> wrote:
> Unsized arrays appearing inside named interface blocks now get a
> proper size assigned by the array_sizing_visitor.
>
> Fixes piglit tests:
> - spec/glsl-1.50/execution/unsized-in-named-interface-block
> - spec/glsl-1.50/execution/unsized-in-named-interface-block-gs
> - spec/glsl-1.50/linker/unsized-in-named-interface-block
> - spec/glsl-1.50/linker/unsized-in-named-interface-block-gs
> - spec/glsl-1.50/linker/unsized-in-unnamed-interface-block-gs (*)
>
> (*) is fixed by dumb luck--support for unsized arrays in unnamed
> interface blocks will come in a later patch.
> ---
>  src/glsl/ir.h       | 16 +++++++++++
>  src/glsl/linker.cpp | 77 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 87 insertions(+), 6 deletions(-)
>
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index 4f63562..16bb76c 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -420,6 +420,22 @@ public:
>        }
>     }
>
> +   /**
> +    * Change this->interface_type on a variable that previously had a
> +    * different interface_type.  This is used during linking to set the size
> +    * of arrays in interface blocks.
> +    */
> +   void change_interface_type(const struct glsl_type *type)
> +   {
> +      if (this->max_ifc_array_access != NULL) {
> +         /* max_ifc_array_access has already been allocated, so make sure the
> +          * new interface has the same number of fields as the old one.
> +          */
> +         assert(this->interface_type->length == type->length);
> +      }
> +      this->interface_type = type;
> +   }
> +
>     const glsl_type *get_interface_type() const
>     {
>        return this->interface_type;
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index c54b704..fe4ccee 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1035,15 +1035,80 @@ class array_sizing_visitor : public ir_hierarchical_visitor {
>  public:
>     virtual ir_visitor_status visit(ir_variable *var)
>     {
> -      if (var->type->is_array() && (var->type->length == 0)) {
> -         const glsl_type *type =
> -            glsl_type::get_array_instance(var->type->fields.array,
> -                                          var->max_array_access + 1);
> -         assert(type != NULL);
> -         var->type = type;
> +      fixup_type(&var->type, var->max_array_access);
> +      if (var->type->is_interface()) {
> +         if (interface_contains_unsized_arrays(var->type)) {
> +            const glsl_type *new_type =
> +               resize_interface_members(var->type, var->max_ifc_array_access);
> +            var->type = new_type;
> +            var->change_interface_type(new_type);
> +         }
> +      } else if (var->type->is_array() &&
> +                 var->type->fields.array->is_interface()) {
> +         if (interface_contains_unsized_arrays(var->type->fields.array)) {
> +            const glsl_type *new_type =
> +               resize_interface_members(var->type->fields.array,
> +                                        var->max_ifc_array_access);
> +            var->change_interface_type(new_type);
> +            var->type =
> +               glsl_type::get_array_instance(new_type, var->type->length);
> +         }
>        }
>        return visit_continue;
>     }
> +
> +private:
> +   /**
> +    * If the type pointed to by \c type represents an unsized array, replace
> +    * it with a sized array whose size is determined by max_array_access.
> +    */
> +   static void fixup_type(const glsl_type **type, unsigned max_array_access)
> +   {
> +      if ((*type)->is_array() && (*type)->length == 0) {
> +         *type = glsl_type::get_array_instance((*type)->fields.array,
> +                                               max_array_access + 1);
> +         assert(*type != NULL);
> +      }
> +   }
> +
> +   /**
> +    * Determine whether the given interface type contains unsized arrays (if
> +    * it doesn't, array_sizing_visitor doesn't need to process it).
> +    */
> +   static bool interface_contains_unsized_arrays(const glsl_type *type)
> +   {
> +      for (unsigned i = 0; i < type->length; i++) {
> +         const glsl_type *elem_type = type->fields.structure[i].type;
> +         if (elem_type->is_array() && elem_type->length == 0)
> +            return true;
> +      }
> +      return false;
> +   }
> +
> +   /**
> +    * Create a new interface type based on the given type, with unsized arrays
> +    * replaced by sized arrays whose size is determined by
> +    * max_ifc_array_access.
> +    */
> +   static const glsl_type *
> +   resize_interface_members(const glsl_type *type,
> +                            const unsigned *max_ifc_array_access)
> +   {
> +      unsigned num_fields = type->length;
> +      glsl_struct_field *fields = new glsl_struct_field[num_fields];

Couldn't you use:
glsl_struct_field fields[num_fields];
to allocate on the stack, and drop the delete below?

> +      memcpy(fields, type->fields.structure,
> +             num_fields * sizeof(*fields));
> +      for (unsigned i = 0; i < num_fields; i++) {
> +         fixup_type(&fields[i].type, max_ifc_array_access[i]);
> +      }
> +      glsl_interface_packing packing =
> +         (glsl_interface_packing) type->interface_packing;
> +      const glsl_type *new_ifc_type =
> +         glsl_type::get_interface_instance(fields, num_fields,
> +                                           packing, type->name);
> +      delete [] fields;
> +      return new_ifc_type;
> +   }
>  };
>
>  /**
> --
> 1.8.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev