[Mesa-dev] [PATCH 4/9] glsl: Add array specifier to ast code

Matt Turner mattst88 at gmail.com
Thu Jan 16 16:41:44 PST 2014


On Wed, Jan 15, 2014 at 10:27 PM, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> Signed-off-by: Timothy Arceri <t_arceri at yahoo.com.au>
> ---
>  src/glsl/ast.h                  |  45 +++++++----
>  src/glsl/ast_array_index.cpp    |  13 +++
>  src/glsl/ast_to_hir.cpp         | 173 +++++++++++++++++++++++++++-------------
>  src/glsl/ast_type.cpp           |   8 +-
>  src/glsl/glsl_parser_extras.cpp |  30 +++----
>  src/glsl/glsl_parser_extras.h   |   2 +
>  6 files changed, 179 insertions(+), 92 deletions(-)
>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index 76911f0..bbae9cd 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -276,6 +276,21 @@ private:
>     bool cons;
>  };
>
> +class ast_array_specifier : public ast_node {
> +public:
> +   ast_array_specifier()
> +      : ast_node()
> +   {
> +      dimension_count = 1;
> +   }
> +
> +   virtual void print(void) const;
> +
> +   unsigned dimension_count;
> +   bool is_unsized_array;
> +   exec_list array_dimensions;
> +};
> +
>  /**
>   * C-style aggregate initialization class
>   *
> @@ -325,14 +340,15 @@ public:
>
>  class ast_declaration : public ast_node {
>  public:
> -   ast_declaration(const char *identifier, bool is_array, ast_expression *array_size,
> -                  ast_expression *initializer);
> +   ast_declaration(const char *identifier, bool is_array,
> +                   ast_array_specifier *array_specifier,
> +                   ast_expression *initializer);
>     virtual void print(void) const;
>
>     const char *identifier;
>
>     bool is_array;
> -   ast_expression *array_size;
> +   ast_array_specifier *array_specifier;
>
>     ast_expression *initializer;
>  };
> @@ -531,7 +547,6 @@ public:
>  };
>
>
> -
>  class ast_type_specifier : public ast_node {
>  public:
>     /**
> @@ -542,9 +557,9 @@ public:
>      * be modified. Zeros the inherited ast_node's fields.
>      */
>     ast_type_specifier(const ast_type_specifier *that, bool is_array,
> -                      ast_expression *array_size)
> +                      ast_array_specifier *array_specifier)
>        : ast_node(), type_name(that->type_name), structure(that->structure),
> -        is_array(is_array), array_size(array_size),
> +        is_array(is_array), array_specifier(array_specifier),
>          default_precision(that->default_precision)
>     {
>        /* empty */
> @@ -553,7 +568,7 @@ public:
>     /** Construct a type specifier from a type name */
>     ast_type_specifier(const char *name)
>        : type_name(name), structure(NULL),
> -       is_array(false), array_size(NULL),
> +       is_array(false), array_specifier(NULL),
>         default_precision(ast_precision_none)
>     {
>        /* empty */
> @@ -562,7 +577,7 @@ public:
>     /** Construct a type specifier from a structure definition */
>     ast_type_specifier(ast_struct_specifier *s)
>        : type_name(s->name), structure(s),
> -       is_array(false), array_size(NULL),
> +       is_array(false), array_specifier(NULL),
>         default_precision(ast_precision_none)
>     {
>        /* empty */
> @@ -580,7 +595,7 @@ public:
>     ast_struct_specifier *structure;
>
>     bool is_array;
> -   ast_expression *array_size;
> +   ast_array_specifier *array_specifier;
>
>     /** For precision statements, this is the given precision; otherwise none. */
>     unsigned default_precision:2;
> @@ -634,7 +649,7 @@ public:
>        type(NULL),
>        identifier(NULL),
>        is_array(false),
> -      array_size(NULL),
> +      array_specifier(NULL),
>        formal_parameter(false),
>        is_void(false)
>     {
> @@ -649,7 +664,7 @@ public:
>     ast_fully_specified_type *type;
>     const char *identifier;
>     bool is_array;
> -   ast_expression *array_size;
> +   ast_array_specifier *array_specifier;
>
>     static void parameters_to_hir(exec_list *ast_parameters,
>                                  bool formal, exec_list *ir_parameters,
> @@ -897,12 +912,12 @@ public:
>     ast_interface_block(ast_type_qualifier layout,
>                         const char *instance_name,
>                         bool is_array,
> -                       ast_expression *array_size)
> +                       ast_array_specifier *array_specifier)
>     : layout(layout), block_name(NULL), instance_name(instance_name),
> -     is_array(is_array), array_size(array_size)
> +     is_array(is_array), array_specifier(array_specifier)
>     {
>        if (!is_array)
> -         assert(array_size == NULL);
> +         assert(array_specifier == NULL);
>     }
>
>     virtual ir_rvalue *hir(exec_list *instructions,
> @@ -937,7 +952,7 @@ public:
>      * If the block is not declared as an array or if the block instance array
>      * is unsized, this field will be \c NULL.
>      */
> -   ast_expression *array_size;
> +   ast_array_specifier *array_specifier;
>  };
>
>
> diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
> index a5f2320..f3b060e 100644
> --- a/src/glsl/ast_array_index.cpp
> +++ b/src/glsl/ast_array_index.cpp
> @@ -25,6 +25,19 @@
>  #include "glsl_types.h"
>  #include "ir.h"
>
> +void
> +ast_array_specifier::print(void) const
> +{
> +   if (this->is_unsized_array) {
> +      printf("[ ] ");
> +   }
> +
> +   foreach_list_typed (ast_node, array_dimension, link, &this->array_dimensions) {
> +      printf("[ ");
> +      array_dimension->print();
> +      printf("] ");
> +   }
> +}
>
>  /**
>   * If \c ir is a reference to an array for which we are tracking the max array
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 4cc8eb1..c0e3443 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -1771,67 +1771,131 @@ ast_compound_statement::hir(exec_list *instructions,
>     return NULL;
>  }
>
> +static const unsigned
> +process_array_size(exec_node *node,
> +                   struct _mesa_glsl_parse_state *state)
> +{
> +   int result = 0;
> +   exec_list dummy_instructions;
> +
> +   ast_node *array_size = exec_node_data(ast_node, node, link);
> +   ir_rvalue *const ir = array_size->hir(& dummy_instructions,
> +                                                   state);
> +   YYLTYPE loc = array_size->get_location();
> +
> +   if (ir != NULL) {
> +      if (!ir->type->is_integer()) {
> +         _mesa_glsl_error(& loc, state,
> +                          "array size must be integer type");
> +      } else if (!ir->type->is_scalar()) {
> +         _mesa_glsl_error(& loc, state,
> +                          "array size must be scalar type");
> +      } else {
> +         ir_constant *const size = ir->constant_expression_value();
> +
> +         if (size == NULL) {
> +            _mesa_glsl_error(& loc, state, "array size must be a "
> +                             "constant valued expression");
> +         } else if (size->value.i[0] <= 0) {
> +            _mesa_glsl_error(& loc, state, "array size must be > 0");
> +         } else {
> +            assert(size->type == ir->type);
> +            result = size->value.u[0];
> +
> +            /* If the array size is const (and we've verified that
> +             * it is) then no instructions should have been emitted
> +             * when we converted it to HIR. If they were emitted,
> +             * then either the array size isn't const after all, or
> +             * we are emitting unnecessary instructions.
> +             */
> +            assert(dummy_instructions.is_empty());
> +         }
> +      }
> +   }
> +   return result;
> +}
>
>  static const glsl_type *
> -process_array_type(YYLTYPE *loc, const glsl_type *base, ast_node *array_size,
> -                  struct _mesa_glsl_parse_state *state)
> +process_array_type(YYLTYPE *loc, const glsl_type *base,
> +                   ast_array_specifier *array_specifier,
> +                   struct _mesa_glsl_parse_state *state)
>  {
> -   unsigned length = 0;
> +   const glsl_type *array_type = NULL;
> +   const glsl_type *element_type = base;
> +   const glsl_type *array_type_temp = element_type;
> +
> +   unsigned outer_dimension_count = 0;
> +   unsigned dimension_count = 1;
>
>     if (base == NULL)
>        return glsl_type::error_type;
>
> -   /* From page 19 (page 25) of the GLSL 1.20 spec:
> -    *
> -    *     "Only one-dimensional arrays may be declared."
> -    */
>     if (base->is_array()) {
> -      _mesa_glsl_error(loc, state,
> -                      "invalid array of `%s' (only one-dimensional arrays "
> -                      "may be declared)",
> -                      base->name);
> -      return glsl_type::error_type;
> +
> +      /* From page 19 (page 25) of the GLSL 1.20 spec:
> +       *
> +       * "Only one-dimensional arrays may be declared."
> +       */
> +      if (!state->ARB_arrays_of_arrays_enable) {
> +         _mesa_glsl_error(loc, state,
> +                          "invalid array of `%s'"
> +                          "#version 120 / GL_ARB_arrays_of_arrays "
> +                          "required for defining arrays of arrays",
> +                          base->name);
> +         return glsl_type::error_type;
> +      }
> +
> +      if (base->length == 0) {
> +         _mesa_glsl_error(loc, state,
> +                          "only the outermost array dimension can "
> +                          "be unsized",
> +                          base->name);
> +         return glsl_type::error_type;
> +      }
> +
> +      /* Settings these variables will cause
> +       * any array dimensions processed with the base type to
> +       * be appended onto the end of the array
> +       */
> +      outer_dimension_count = base->dimension_count;
> +      dimension_count = base->dimension_count + 1;
> +      element_type = base->element_type();
>     }
>
> -   if (array_size != NULL) {
> -      exec_list dummy_instructions;
> -      ir_rvalue *const ir = array_size->hir(& dummy_instructions, state);
> -      YYLTYPE loc = array_size->get_location();
> +   if (array_specifier != NULL) {
>
> -      if (ir != NULL) {
> -        if (!ir->type->is_integer()) {
> -           _mesa_glsl_error(& loc, state, "array size must be integer type");
> -        } else if (!ir->type->is_scalar()) {
> -           _mesa_glsl_error(& loc, state, "array size must be scalar type");
> -        } else {
> -           ir_constant *const size = ir->constant_expression_value();
> -
> -           if (size == NULL) {
> -              _mesa_glsl_error(& loc, state, "array size must be a "
> -                               "constant valued expression");
> -           } else if (size->value.i[0] <= 0) {
> -              _mesa_glsl_error(& loc, state, "array size must be > 0");
> -           } else {
> -              assert(size->type == ir->type);
> -              length = size->value.u[0];
> -
> -               /* If the array size is const (and we've verified that
> -                * it is) then no instructions should have been emitted
> -                * when we converted it to HIR.  If they were emitted,
> -                * then either the array size isn't const after all, or
> -                * we are emitting unnecessary instructions.
> -                */
> -               assert(dummy_instructions.is_empty());
> -           }
> -        }
> +      exec_node *node = array_specifier->array_dimensions.tail_pred;
> +      outer_dimension_count += array_specifier->dimension_count;
> +
> +      unsigned array_size;
> +      for (/* nothing */; !node->is_head_sentinel(); node = node->prev) {
> +         array_size = process_array_size(node, state);
> +         array_type_temp = glsl_type::get_array_instance(array_type_temp,
> +                                                         array_size,
> +                                                         dimension_count);
> +         dimension_count++;
> +      }
> +
> +      if (array_specifier->is_unsized_array) {
> +         array_type_temp = glsl_type::get_array_instance(array_type_temp,
> +                                                         0,
> +                                                         dimension_count);
> +      }
> +
> +      if (array_type_temp == element_type) {
> +         /* we found no array sizes */
> +         array_type = NULL;
> +      } else {
> +         array_type = array_type_temp;
>        }
>     }
>
> -   const glsl_type *array_type = glsl_type::get_array_instance(base, length);
> +   /* make sure the ast count and the current count match */
> +   assert(dimension_count == outer_dimension_count);
> +
>     return array_type != NULL ? array_type : glsl_type::error_type;
>  }
>
> -
>  const glsl_type *
>  ast_type_specifier::glsl_type(const char **name,
>                               struct _mesa_glsl_parse_state *state) const
> @@ -1843,7 +1907,7 @@ 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_size, state);
> +      type = process_array_type(&loc, type, this->array_specifier, state);
>     }
>
>     return type;
> @@ -2824,7 +2888,7 @@ ast_declarator_list::hir(exec_list *instructions,
>
>        foreach_list_typed (ast_declaration, decl, link, &this->declarations) {
>          assert(!decl->is_array);
> -        assert(decl->array_size == NULL);
> +        assert(decl->array_specifier == NULL);
>          assert(decl->initializer == NULL);
>
>          ir_variable *const earlier =
> @@ -2960,7 +3024,7 @@ ast_declarator_list::hir(exec_list *instructions,
>        }
>
>        if (decl->is_array) {
> -        var_type = process_array_type(&loc, decl_type, decl->array_size,
> +        var_type = process_array_type(&loc, decl_type, decl->array_specifier,
>                                        state);
>          if (var_type->is_error())
>             continue;
> @@ -3524,7 +3588,7 @@ 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_size, state);
> +      type = process_array_type(&loc, type, this->array_specifier, state);
>     }
>
>     if (!type->is_error() && type->is_unsized_array()) {
> @@ -4654,8 +4718,8 @@ ast_process_structure_or_interface_block(exec_list *instructions,
>           }
>
>          if (decl->is_array) {
> -           field_type = process_array_type(&loc, decl_type, decl->array_size,
> -                                           state);
> +           field_type = process_array_type(&loc, decl_type,
> +                                            decl->array_specifier, state);
>          }
>           fields[i].type = field_type;
>          fields[i].name = decl->identifier;
> @@ -5019,6 +5083,10 @@ ast_interface_block::hir(exec_list *instructions,
>        ir_variable *var;
>
>        if (this->is_array) {
> +
> +         const glsl_type *block_array_type =
> +            process_array_type(&loc, block_type, this->array_specifier, state);
> +
>           /* Section 4.3.7 (Interface Blocks) of the GLSL 1.50 spec says:
>            *
>            *     For uniform blocks declared an array, each individual array
> @@ -5038,7 +5106,7 @@ ast_interface_block::hir(exec_list *instructions,
>            * interface array size *doesn't* need to be specified is on a
>            * geometry shader input.
>            */
> -         if (this->array_size == NULL &&
> +         if (block_array_type->is_unsized_array() &&
>               (state->stage != MESA_SHADER_GEOMETRY || !this->layout.flags.q.in)) {
>              _mesa_glsl_error(&loc, state,
>                               "only geometry shader inputs may be unsized "
> @@ -5046,9 +5114,6 @@ ast_interface_block::hir(exec_list *instructions,
>
>           }
>
> -         const glsl_type *block_array_type =
> -            process_array_type(&loc, block_type, this->array_size, state);
> -
>           var = new(state) ir_variable(block_array_type,
>                                        this->instance_name,
>                                        var_mode);
> diff --git a/src/glsl/ast_type.cpp b/src/glsl/ast_type.cpp
> index d758bfa..ac48597 100644
> --- a/src/glsl/ast_type.cpp
> +++ b/src/glsl/ast_type.cpp
> @@ -33,13 +33,9 @@ ast_type_specifier::print(void) const
>     }
>
>     if (is_array) {
> -      printf("[ ");
> -
> -      if (array_size) {
> -        array_size->print();
> +      if (array_specifier) {
> +        array_specifier->print();
>        }
> -
> -      printf("] ");
>     }
>  }
>
> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
> index 21dc3ab..92076b5 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -484,6 +484,7 @@ struct _mesa_glsl_extension {
>  static const _mesa_glsl_extension _mesa_glsl_supported_extensions[] = {
>     /*                                  API availability */
>     /* name                             GL     ES         supported flag */
> +   EXT(ARB_arrays_of_arrays,           true,  false,     ARB_arrays_of_arrays),
>     EXT(ARB_conservative_depth,         true,  false,     ARB_conservative_depth),
>     EXT(ARB_draw_buffers,               true,  false,     dummy_true),
>     EXT(ARB_draw_instanced,             true,  false,     ARB_draw_instanced),
> @@ -769,7 +770,7 @@ _mesa_ast_set_aggregate_type(const ast_type_specifier *type,
>                                                    link);
>
>              bool is_array = decl_list->type->specifier->is_array;
> -            ast_expression *array_size = decl_list->type->specifier->array_size;
> +            ast_array_specifier *array_specifier = decl_list->type->specifier->array_specifier;
>
>              /* Recognize variable declarations with the bracketed size attached
>               * to the type rather than the variable name as arrays. E.g.,
> @@ -777,19 +778,18 @@ _mesa_ast_set_aggregate_type(const ast_type_specifier *type,
>               *     float a[2];
>               *     float[2] b;
>               *
> -             * are both arrays, but <a>'s array_size is decl->array_size, while
> -             * <b>'s array_size is decl_list->type->specifier->array_size.
> +             * 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) {
> -               /* FINISHME: Update when ARB_array_of_arrays is supported. */

There's another identical FINISHME in this function that I don't see
in this patch.

>                 is_array = decl->is_array;
> -               array_size = decl->array_size;
> +               array_specifier = decl->array_specifier;
>              }

I don't think this is going to work for

   float[2] a[3]

will it? is_array will be true, but the whole array_specifier won't be complete.

>
>              /* Declaration shadows the <type> parameter. */
>              ast_type_specifier *type =
>                 new(ctx) ast_type_specifier(decl_list->type->specifier,
> -                                           is_array, array_size);
> +                                           is_array, array_specifier);
>
>              if (expr->oper == ast_aggregate)
>                 _mesa_ast_set_aggregate_type(type, expr, state);
> @@ -876,15 +876,11 @@ ast_node::ast_node(void)
>
>
>  static void
> -ast_opt_array_size_print(bool is_array, const ast_expression *array_size)
> +ast_opt_array_dimensions_print(bool is_array, const ast_array_specifier *array_specifier)
>  {
>     if (is_array) {
> -      printf("[ ");
> -
> -      if (array_size)
> -        array_size->print();
> -
> -      printf("] ");
> +      if (array_specifier)
> +         array_specifier->print();
>     }
>  }
>
> @@ -1108,7 +1104,7 @@ ast_parameter_declarator::print(void) const
>     type->print();
>     if (identifier)
>        printf("%s ", identifier);
> -   ast_opt_array_size_print(is_array, array_size);
> +   ast_opt_array_dimensions_print(is_array, array_specifier);
>  }
>
>
> @@ -1124,7 +1120,7 @@ void
>  ast_declaration::print(void) const
>  {
>     printf("%s ", identifier);
> -   ast_opt_array_size_print(is_array, array_size);
> +   ast_opt_array_dimensions_print(is_array, array_specifier);
>
>     if (initializer) {
>        printf("= ");
> @@ -1134,12 +1130,12 @@ ast_declaration::print(void) const
>
>
>  ast_declaration::ast_declaration(const char *identifier, bool is_array,
> -                                ast_expression *array_size,
> +                                ast_array_specifier *array_specifier,
>                                  ast_expression *initializer)
>  {
>     this->identifier = identifier;
>     this->is_array = is_array;
> -   this->array_size = array_size;
> +   this->array_specifier = array_specifier;
>     this->initializer = initializer;
>  }
>
> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
> index 2444a96..1478f00 100644
> --- a/src/glsl/glsl_parser_extras.h
> +++ b/src/glsl/glsl_parser_extras.h
> @@ -294,6 +294,8 @@ struct _mesa_glsl_parse_state {
>      * \name Enable bits for GLSL extensions
>      */
>     /*@{*/
> +   bool ARB_arrays_of_arrays_enable;
> +   bool ARB_arrays_of_arrays_warn;
>     bool ARB_draw_buffers_enable;
>     bool ARB_draw_buffers_warn;
>     bool ARB_draw_instanced_enable;
> --
> 1.8.3.1


More information about the mesa-dev mailing list