[Mesa-dev] [PATCH V2 5/8] glsl: Aggregate initializer support for arrays of array

Paul Berry stereotype441 at gmail.com
Tue Jan 21 18:05:54 PST 2014


On 21 January 2014 04:19, Timothy Arceri <t_arceri at yahoo.com.au> wrote:

> Signed-off-by: Timothy Arceri <t_arceri at yahoo.com.au>
> ---
>  src/glsl/ast.h                  | 19 +++++++++++-
>  src/glsl/ast_function.cpp       | 14 +++++++--
>  src/glsl/ast_to_hir.cpp         | 29 +++++++++++++-----
>  src/glsl/glsl_parser.yy         | 36 +++++++++++++++++++++-
>  src/glsl/glsl_parser_extras.cpp | 68
> +++++++++++++++++++++++++++++++----------
>  5 files changed, 139 insertions(+), 27 deletions(-)
>

I believe I've found an easier solution to this: please see "[PATCH] glsl:
Simplify aggregate type inference to prepare for ARB_arrays_of_arrays.",
which I believe makes this patch unnecessary.


>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index 4dda32e..8bccca7 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -308,10 +308,16 @@ public:
>        : ast_expression(ast_aggregate, NULL, NULL, NULL),
>          constructor_type(NULL)
>     {
> -      /* empty */
> +      array_dimension = 1;
> +      is_array = false;
>     }
>
>     ast_type_specifier *constructor_type;
> +   unsigned array_dimension;
> +
> +   ast_array_specifier *array_specifier;
> +   bool is_array;
> +
>     virtual ir_rvalue *hir(exec_list *instructions,
>                            struct _mesa_glsl_parse_state *state);
>  };
> @@ -588,6 +594,11 @@ public:
>                                      struct _mesa_glsl_parse_state *state)
>        const;
>
> +   const struct glsl_type *glsl_type(const char **name,
> +                                     struct _mesa_glsl_parse_state *state,
> +                                     bool skip_outer_dimension)
> +      const;
> +
>     virtual void print(void) const;
>
>     ir_rvalue *hir(exec_list *, struct _mesa_glsl_parse_state *);
> @@ -1005,4 +1016,10 @@ extern void
>  check_builtin_array_max_size(const char *name, unsigned size,
>                               YYLTYPE loc, struct _mesa_glsl_parse_state
> *state);
>
> +extern const glsl_type *
> +process_array_type(YYLTYPE *loc, const glsl_type *base,
> +                   ast_array_specifier *array_specifier,
> +                  struct _mesa_glsl_parse_state *state,
> +                   bool skip_first_dim);
> +
>  #endif /* AST_H */
> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
> index 2d05d07..3b38cb6 100644
> --- a/src/glsl/ast_function.cpp
> +++ b/src/glsl/ast_function.cpp
> @@ -1693,8 +1693,18 @@ ast_aggregate_initializer::hir(exec_list
> *instructions,
>        _mesa_glsl_error(&loc, state, "type of C-style initializer
> unknown");
>        return ir_rvalue::error_value(ctx);
>     }
> -   const glsl_type *const constructor_type =
> -      this->constructor_type->glsl_type(&name, state);
> +   const glsl_type *constructor_type =
> +      this->constructor_type->glsl_type(&name, state,
> +                                        this->array_dimension > 1 &&
> !this->is_array);
> +
> +   /* This only handles "vec4 foo[..]".  The earlier
> constructor_type->glsl_type(...)
> +    * call already handled the "vec4[..] foo" case.
> +    */
> +   if (this->is_array) {
> +      constructor_type =
> +         process_array_type(&loc, constructor_type, this->array_specifier,
> +                            state, this->array_dimension > 1);
> +   }
>
>     if (!state->ARB_shading_language_420pack_enable) {
>        _mesa_glsl_error(&loc, state, "C-style initialization requires the "
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index d02c9ff..226d128 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -1817,10 +1817,11 @@ process_array_size(exec_node *node,
>     return result;
>  }
>
> -static const glsl_type *
> +const glsl_type *
>  process_array_type(YYLTYPE *loc, const glsl_type *base,
>                     ast_array_specifier *array_specifier,
> -                   struct _mesa_glsl_parse_state *state)
> +                   struct _mesa_glsl_parse_state *state,
> +                   bool skip_first_dim)
>  {
>     const glsl_type *array_type = NULL;
>     const glsl_type *element_type = base;
> @@ -1865,6 +1866,8 @@ process_array_type(YYLTYPE *loc, const glsl_type
> *base,
>
>        unsigned array_size;
>        for (/* nothing */; !node->is_head_sentinel(); node = node->prev) {
> +         if (skip_first_dim && node->prev->is_head_sentinel())
> +            break;
>           array_size = process_array_size(node, state);
>           array_type_temp = glsl_type::get_array_instance(array_type_temp,
>                                                           array_size);
> @@ -1891,6 +1894,14 @@ const glsl_type *
>  ast_type_specifier::glsl_type(const char **name,
>                               struct _mesa_glsl_parse_state *state) const
>  {
> +   return this->glsl_type(name, state, false);
> +}
> +
> +const glsl_type *
> +ast_type_specifier::glsl_type(const char **name,
> +                             struct _mesa_glsl_parse_state *state,
> +                              bool skip_outer_dim) const
> +{
>     const struct glsl_type *type;
>
>     type = state->symbols->get_type(this->type_name);
> @@ -1898,7 +1909,8 @@ ast_type_specifier::glsl_type(const char **name,
>
>     if (this->is_array) {
>        YYLTYPE loc = this->get_location();
> -      type = process_array_type(&loc, type, this->array_specifier, state);
> +      type = process_array_type(&loc, type, this->array_specifier,
> +                                state, skip_outer_dim);
>     }
>
>     return type;
> @@ -3016,7 +3028,7 @@ ast_declarator_list::hir(exec_list *instructions,
>
>        if (decl->is_array) {
>          var_type = process_array_type(&loc, decl_type,
> decl->array_specifier,
> -                                      state);
> +                                      state, false);
>          if (var_type->is_error())
>             continue;
>        } else {
> @@ -3579,7 +3591,8 @@ ast_parameter_declarator::hir(exec_list
> *instructions,
>      * call already handled the "vec4[..] foo" case.
>      */
>     if (this->is_array) {
> -      type = process_array_type(&loc, type, this->array_specifier, state);
> +      type = process_array_type(&loc, type, this->array_specifier,
> +                                state, false);
>     }
>
>     if (!type->is_error() && type->is_unsized_array()) {
> @@ -4710,7 +4723,8 @@ ast_process_structure_or_interface_block(exec_list
> *instructions,
>
>          if (decl->is_array) {
>             field_type = process_array_type(&loc, decl_type,
> -                                            decl->array_specifier, state);
> +                                            decl->array_specifier,
> +                                            state, false);
>          }
>           fields[i].type = field_type;
>          fields[i].name = decl->identifier;
> @@ -5076,7 +5090,8 @@ ast_interface_block::hir(exec_list *instructions,
>        if (this->is_array) {
>
>           const glsl_type *block_array_type =
> -            process_array_type(&loc, block_type, this->array_specifier,
> state);
> +            process_array_type(&loc, block_type, this->array_specifier,
> +                               state, false);
>
>           /* Section 4.3.7 (Interface Blocks) of the GLSL 1.50 spec says:
>            *
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index 6d63668..9461e73 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -1007,6 +1007,23 @@ init_declarator_list:
>        if ($6->oper == ast_aggregate) {
>           ast_aggregate_initializer *ai = (ast_aggregate_initializer *)$6;
>           ast_type_specifier *type = new(ctx)
> ast_type_specifier($1->type->specifier, true, $4);
> +         if ($1->type->specifier->array_specifier != NULL) {
> +            ai->array_specifier = $4;
> +            ai->is_array = true;
> +
> +            type =
> +             new(ctx) ast_type_specifier($1->type->specifier,
> +                                         $1->type->specifier->is_array,
> +
> $1->type->specifier->array_specifier);
> +         } else {
> +            ai->array_specifier = NULL;
> +            ai->is_array = false;
> +
> +            type =
> +             new(ctx) ast_type_specifier($1->type->specifier,
> +                                         true,
> +                                         $4);
> +         }
>           _mesa_ast_set_aggregate_type(type, ai, state);
>        }
>     }
> @@ -1063,7 +1080,24 @@ single_declaration:
>        $$->declarations.push_tail(&decl->link);
>        if ($5->oper == ast_aggregate) {
>           ast_aggregate_initializer *ai = (ast_aggregate_initializer *)$5;
> -         ast_type_specifier *type = new(ctx)
> ast_type_specifier($1->specifier, true, $3);
> +         ast_type_specifier *type;
> +         if ($1->specifier->array_specifier != NULL) {
> +            ai->array_specifier = $3;
> +            ai->is_array = true;
> +
> +            type =
> +             new(ctx) ast_type_specifier($1->specifier,
> +                                         $1->specifier->is_array,
> +                                         $1->specifier->array_specifier);
> +         } else {
> +            ai->array_specifier = NULL;
> +            ai->is_array = false;
> +
> +            type =
> +             new(ctx) ast_type_specifier($1->specifier,
> +                                         true,
> +                                         $3);
> +         }
>           _mesa_ast_set_aggregate_type(type, ai, state);
>        }
>     }
> diff --git a/src/glsl/glsl_parser_extras.cpp
> b/src/glsl/glsl_parser_extras.cpp
> index 92076b5..43a23cc 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -720,16 +720,26 @@ _mesa_ast_set_aggregate_type(const
> ast_type_specifier *type,
>
>     /* If the aggregate is an array, recursively set its elements' types.
> */
>     if (type->is_array) {
> -      /* We want to set the element type which is not an array itself, so
> make
> -       * a copy of the array type and set its is_array field to false.
> -       *
> -       * E.g., if <type> if struct S[2] we want to set each element's
> type to
> -       * struct S.
> -       *
> -       * FINISHME: Update when ARB_array_of_arrays is supported.
> -       */
> -      const ast_type_specifier *non_array_type =
> -         new(ctx) ast_type_specifier(type, false, NULL);
> +
> +      const ast_type_specifier *element_type;
> +      unsigned dimension_count = type->array_specifier->dimension_count;
> +
> +      if (ai->is_array)
> +         dimension_count += ai->array_specifier->dimension_count;
> +
> +      if (dimension_count == ai->array_dimension ||
> +          !state->ARB_arrays_of_arrays_enable) {
> +
> +         /* We want to set the element type which is not an array itself,
> so make
> +          * a copy of the array type and set its is_array field to false.
> +          *
> +          * E.g., if <type> if struct S[2] we want to set each element's
> type to
> +          * struct S.
> +          */
> +         element_type = new(ctx) ast_type_specifier(type, false, NULL);
> +      } else {
> +         element_type = type;
> +      }
>
>        for (exec_node *expr_node = ai->expressions.head;
>             !expr_node->is_tail_sentinel();
> @@ -737,8 +747,23 @@ _mesa_ast_set_aggregate_type(const ast_type_specifier
> *type,
>           ast_expression *expr = exec_node_data(ast_expression, expr_node,
>                                                 link);
>
> -         if (expr->oper == ast_aggregate)
> -            _mesa_ast_set_aggregate_type(non_array_type, expr, state);
> +         if (expr->oper == ast_aggregate) {
> +            /* copy some values needed to process initialization of
> +             * arrays of arrays e.g
> +             * vec4[2] a[2] = {
> +             *    { vec4(1.0), vec4(1.0) },
> +             *    { vec4(1.0), vec4(1.0) }};
> +             */
> +            ast_aggregate_initializer *sub_ai =
> (ast_aggregate_initializer *)expr;
> +            if (ai->array_dimension != dimension_count) {
> +               sub_ai->array_dimension = ai->array_dimension + 1;
> +               if (ai->is_array) {
> +                  sub_ai->is_array = ai->is_array;
> +                  sub_ai->array_specifier = ai->array_specifier;
> +               }
> +            }
> +            _mesa_ast_set_aggregate_type(element_type, (ast_expression
> *)sub_ai, state);
> +         }
>        }
>
>     /* If the aggregate is a struct, recursively set its fields' types. */
> @@ -770,6 +795,7 @@ _mesa_ast_set_aggregate_type(const ast_type_specifier
> *type,
>                                                    link);
>
>              bool is_array = decl_list->type->specifier->is_array;
> +            bool set_aggr_array = false;
>              ast_array_specifier *array_specifier =
> decl_list->type->specifier->array_specifier;
>
>              /* Recognize variable declarations with the bracketed size
> attached
> @@ -778,12 +804,16 @@ _mesa_ast_set_aggregate_type(const
> ast_type_specifier *type,
>               *     float a[2];
>               *     float[2] b;
>               *
> -             * are both arrays, but <a>'s array_specifier is
> decl->array_specifier, while
> -             * <b>'s array_specifier is
> decl_list->type->specifier->array_specifier.
> +             * are both arrays, but <a>'s array_specifier is
> +             * decl->array_specifier, while <b>'s array_specifier is
> +             * decl_list->type->specifier->array_specifier.
>               */
>              if (!is_array) {
>                 is_array = decl->is_array;
>                 array_specifier = decl->array_specifier;
> +            } else if (decl_list->type->specifier->is_array &&
> +                       state->ARB_arrays_of_arrays_enable) {
> +               set_aggr_array = true;
>              }
>
>              /* Declaration shadows the <type> parameter. */
> @@ -791,8 +821,14 @@ _mesa_ast_set_aggregate_type(const ast_type_specifier
> *type,
>                 new(ctx) ast_type_specifier(decl_list->type->specifier,
>                                             is_array, array_specifier);
>
> -            if (expr->oper == ast_aggregate)
> -               _mesa_ast_set_aggregate_type(type, expr, state);
> +            if (expr->oper == ast_aggregate) {
> +               ast_aggregate_initializer *sub_ai =
> (ast_aggregate_initializer *)expr;
> +               if (set_aggr_array) {
> +                  sub_ai->is_array = decl->is_array;
> +                  sub_ai->array_specifier = decl->array_specifier;
> +               }
> +               _mesa_ast_set_aggregate_type(type, (ast_expression
> *)sub_ai, state);
> +            }
>           }
>        }
>     } else {
> --
> 1.8.3.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140121/536e626e/attachment-0001.html>


More information about the mesa-dev mailing list