glsl: Simplify aggregate type inference to prepare for ARB_arrays_of_arrays.

Most of the time it is not necessary to perform type inference to
compile GLSL; the type of every expression can be inferred from the
contents of the expression itself (and previous type declarations).
The exception is aggregate initializers: their type is determined by
the LHS of the variable being assigned to.  For example, in the
statement:

   mat2 foo = { { 1, 2 }, { 3, 4 } };

the type of { 1, 2 } is only known to be vec2 (as opposed to, say,
ivec2, uvec2, int[2], or a struct) because of the fact that the result
is being assigned to a mat2.

Previous to this patch, we handled this situation by doing some type
inference during parsing: when parsing a declaration like the one
above, we would call _mesa_set_aggregate_type(), which would infer the
type of each aggregate initializer and store it in the corresponding
ast_aggregate_initializer::constructor_type field.  Since this
happened at parse time, we couldn't do the type inference using
glsl_type objects; we had to use ast_type_specifiers, which are much
more awkward to work with.  Things are about to get more complicated
when we add support for ARB_arrays_of_arrays.

This patch simplifies things by postponing the call to
_mesa_set_aggregate_type() until ast-to-hir time, when we have access
to glsl_type objects.  As a side benefit, we only need to have one
call to _mesa_set_aggregate_type() now, instead of six.

Reviewed-by: Matt Turner <mattst88@gmail.com>

diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 76911f056d34f148e9f2d76b234df9cfcb84dd2e..b24052bfd04f092a57e8582da3ab2edfffc1a146 100644 (file)
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -296,7 +296,16 @@ public:
       /* empty */
    }
 
-   ast_type_specifier *constructor_type;
+   /**
+    * glsl_type of the aggregate, which is inferred from the LHS of whatever
+    * the aggregate is being used to initialize.  This can't be inferred at
+    * parse time (since the parser deals with ast_type_specifiers, not
+    * glsl_types), so the parser leaves it NULL.  However, the ast-to-hir
+    * conversion code makes sure to fill it in with the appropriate type
+    * before hir() is called.
+    */
+   const glsl_type *constructor_type;
+
    virtual ir_rvalue *hir(exec_list *instructions,
                           struct _mesa_glsl_parse_state *state);
 };
@@ -978,9 +987,8 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,
                             YYLTYPE &loc, YYLTYPE &idx_loc);
 
 extern void
-_mesa_ast_set_aggregate_type(const ast_type_specifier *type,
-                             ast_expression *expr,
-                             _mesa_glsl_parse_state *state);
+_mesa_ast_set_aggregate_type(const glsl_type *type,
+                             ast_expression *expr);
 
 void
 emit_function(_mesa_glsl_parse_state *state, ir_function *f);

diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index 2d05d0723411e0ddf4f285829829370ff6cc3446..4c5b0e4aa90ae932960ad8ad6666ffb8e1747601 100644 (file)
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -1687,14 +1687,12 @@ ast_aggregate_initializer::hir(exec_list *instructions,
 {
    void *ctx = state;
    YYLTYPE loc = this->get_location();
-   const char *name;
 
    if (!this->constructor_type) {
       _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 *const constructor_type = this->constructor_type;
 
    if (!state->ARB_shading_language_420pack_enable) {
       _mesa_glsl_error(&loc, state, "C-style initialization requires the "
@@ -1702,12 +1700,12 @@ ast_aggregate_initializer::hir(exec_list *instructions,
       return ir_rvalue::error_value(ctx);
    }
 
-   if (this->constructor_type->is_array) {
+   if (constructor_type->is_array()) {
       return process_array_constructor(instructions, constructor_type, &loc,
                                        &this->expressions, state);
    }
 
-   if (this->constructor_type->structure) {
+   if (constructor_type->is_record()) {
       return process_record_constructor(instructions, constructor_type, &loc,
                                         &this->expressions, state);
    }

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 4cc8eb18dc8c04e80e4793cdb6de5051dce5d872..8d13610e426a7ccbb9df389baaa14aa852ae4b8a 100644 (file)
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2593,6 +2593,13 @@ process_initializer(ir_variable *var, ast_declaration *decl,
                       ? "attribute" : "varying");
    }
 
+   /* If the initializer is an ast_aggregate_initializer, recursively store
+    * type information from the LHS into it, so that its hir() function can do
+    * type checking.
+    */
+   if (decl->initializer->oper == ast_aggregate)
+      _mesa_ast_set_aggregate_type(var->type, decl->initializer);
+
    ir_dereference *const lhs = new(state) ir_dereference_variable(var);
    ir_rvalue *rhs = decl->initializer->hir(initializer_instructions,
                                           state);

diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index 1c56d6f145ada316f1791600a3b2bc072f2219db..5451b764a7797f9f26f96b3161a76e6d5cbe9cf6 100644 (file)
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -1012,11 +1012,6 @@ init_declarator_list:
       $$ = $1;
       $$->declarations.push_tail(&decl->link);
       state->symbols->add_variable(new(state) ir_variable(NULL, $3, ir_var_auto));
-      if ($7->oper == ast_aggregate) {
-         ast_aggregate_initializer *ai = (ast_aggregate_initializer *)$7;
-         ast_type_specifier *type = new(ctx) ast_type_specifier($1->type->specifier, true, NULL);
-         _mesa_ast_set_aggregate_type(type, ai, state);
-      }
    }
    | init_declarator_list ',' any_identifier '[' constant_expression ']' '=' initializer
    {
@@ -1027,11 +1022,6 @@ init_declarator_list:
       $$ = $1;
       $$->declarations.push_tail(&decl->link);
       state->symbols->add_variable(new(state) ir_variable(NULL, $3, ir_var_auto));
-      if ($8->oper == ast_aggregate) {
-         ast_aggregate_initializer *ai = (ast_aggregate_initializer *)$8;
-         ast_type_specifier *type = new(ctx) ast_type_specifier($1->type->specifier, true, $5);
-         _mesa_ast_set_aggregate_type(type, ai, state);
-      }
    }
    | init_declarator_list ',' any_identifier '=' initializer
    {
@@ -1042,10 +1032,6 @@ init_declarator_list:
       $$ = $1;
       $$->declarations.push_tail(&decl->link);
       state->symbols->add_variable(new(state) ir_variable(NULL, $3, ir_var_auto));
-      if ($5->oper == ast_aggregate) {
-         ast_aggregate_initializer *ai = (ast_aggregate_initializer *)$5;
-         _mesa_ast_set_aggregate_type($1->type->specifier, ai, state);
-      }
    }
    ;
 
@@ -1093,11 +1079,6 @@ single_declaration:
       $$ = new(ctx) ast_declarator_list($1);
       $$->set_location(yylloc);
       $$->declarations.push_tail(&decl->link);
-      if ($6->oper == ast_aggregate) {
-         ast_aggregate_initializer *ai = (ast_aggregate_initializer *)$6;
-         ast_type_specifier *type = new(ctx) ast_type_specifier($1->specifier, true, NULL);
-         _mesa_ast_set_aggregate_type(type, ai, state);
-      }
    }
    | fully_specified_type any_identifier '[' constant_expression ']' '=' initializer
    {
@@ -1107,11 +1088,6 @@ single_declaration:
       $$ = new(ctx) ast_declarator_list($1);
       $$->set_location(yylloc);
       $$->declarations.push_tail(&decl->link);
-      if ($7->oper == ast_aggregate) {
-         ast_aggregate_initializer *ai = (ast_aggregate_initializer *)$7;
-         ast_type_specifier *type = new(ctx) ast_type_specifier($1->specifier, true, $4);
-         _mesa_ast_set_aggregate_type(type, ai, state);
-      }
    }
    | fully_specified_type any_identifier '=' initializer
    {
@@ -1121,9 +1097,6 @@ single_declaration:
       $$ = new(ctx) ast_declarator_list($1);
       $$->set_location(yylloc);
       $$->declarations.push_tail(&decl->link);
-      if ($4->oper == ast_aggregate) {
-         _mesa_ast_set_aggregate_type($1->specifier, $4, state);
-      }
    }
    | INVARIANT variable_identifier // Vertex only.
    {

diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
index 9a1e0a249ed0df08c9fa3519cb54701cfd9fc7d9..345e426b75a3ba9934c4456a0991784925f49a5e 100644 (file)
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -634,25 +634,6 @@ _mesa_glsl_process_extension(const char *name, YYLTYPE *name_locp,
 }
 
 
-/**
- * Returns the name of the type of a column of a matrix. E.g.,
- *
- *    "mat3"   -> "vec3"
- *    "mat4x2" -> "vec2"
- */
-static const char *
-_mesa_ast_get_matrix_column_type_name(const char *matrix_type_name)
-{
-   static const char *vec_name[] = { "vec2", "vec3", "vec4" };
-
-   /* The number of elements in a row of a matrix is specified by the last
-    * character of the matrix type name.
-    */
-   long rows = strtol(matrix_type_name + strlen(matrix_type_name) - 1,
-                      NULL, 10);
-   return vec_name[rows - 2];
-}
-
 /**
  * Recurses through <type> and <expr> if <expr> is an aggregate initializer
  * and sets <expr>'s <constructor_type> field to <type>. Gives later functions
@@ -700,37 +681,19 @@ _mesa_ast_get_matrix_column_type_name(const char *matrix_type_name)
  * doesn't contain sufficient information to determine if the types match.
  */
 void
-_mesa_ast_set_aggregate_type(const ast_type_specifier *type,
-                             ast_expression *expr,
-                             _mesa_glsl_parse_state *state)
+_mesa_ast_set_aggregate_type(const glsl_type *type,
+                             ast_expression *expr)
 {
-   void *ctx = state;
    ast_aggregate_initializer *ai = (ast_aggregate_initializer *)expr;
-   ai->constructor_type = (ast_type_specifier *)type;
-
-   bool is_declaration = ai->constructor_type->structure != NULL;
-   if (!is_declaration) {
-      /* Look up <type> name in the symbol table to see if it's a struct. */
-      const ast_type_specifier *struct_type =
-         state->symbols->get_type_ast(type->type_name);
-      ai->constructor_type->structure =
-         struct_type ? new(ctx) ast_struct_specifier(*struct_type->structure)
-                     : NULL;
-   }
+   ai->constructor_type = 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.
+   if (type->is_array()) {
+      /* Each array element has the type type->element_type().
        *
        * 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);
-
       for (exec_node *expr_node = ai->expressions.head;
            !expr_node->is_tail_sentinel();
            expr_node = expr_node->next) {
@@ -738,84 +701,33 @@ _mesa_ast_set_aggregate_type(const ast_type_specifier *type,
                                                link);
 
          if (expr->oper == ast_aggregate)
-            _mesa_ast_set_aggregate_type(non_array_type, expr, state);
+            _mesa_ast_set_aggregate_type(type->element_type(), expr);
       }
 
    /* If the aggregate is a struct, recursively set its fields' types. */
-   } else if (ai->constructor_type->structure) {
-      ai->constructor_type->structure->is_declaration = is_declaration;
+   } else if (type->is_record()) {
       exec_node *expr_node = ai->expressions.head;
 
-      /* Iterate through the struct's fields' declarations. E.g., iterate from
-       * "float a, b" to "int c" in the struct below.
-       *
-       *     struct {
-       *         float a, b;
-       *         int c;
-       *     } s;
-       */
-      for (exec_node *decl_list_node =
-              ai->constructor_type->structure->declarations.head;
-           !decl_list_node->is_tail_sentinel();
-           decl_list_node = decl_list_node->next) {
-         ast_declarator_list *decl_list = exec_node_data(ast_declarator_list,
-                                                         decl_list_node, link);
-
-         for (exec_node *decl_node = decl_list->declarations.head;
-              !decl_node->is_tail_sentinel() && !expr_node->is_tail_sentinel();
-              decl_node = decl_node->next, expr_node = expr_node->next) {
-            ast_declaration *decl = exec_node_data(ast_declaration, decl_node,
-                                                   link);
-            ast_expression *expr = exec_node_data(ast_expression, expr_node,
-                                                  link);
-
-            bool is_array = decl_list->type->specifier->is_array;
-            ast_expression *array_size = decl_list->type->specifier->array_size;
-
-            /* Recognize variable declarations with the bracketed size attached
-             * to the type rather than the variable name as arrays. E.g.,
-             *
-             *     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.
-             */
-            if (!is_array) {
-               /* FINISHME: Update when ARB_array_of_arrays is supported. */
-               is_array = decl->is_array;
-               array_size = decl->array_size;
-            }
-
-            /* Declaration shadows the <type> parameter. */
-            ast_type_specifier *type =
-               new(ctx) ast_type_specifier(decl_list->type->specifier,
-                                           is_array, array_size);
+      /* Iterate through the struct's fields. */
+      for (unsigned i = 0; !expr_node->is_tail_sentinel() && i < type->length;
+           i++, expr_node = expr_node->next) {
+         ast_expression *expr = exec_node_data(ast_expression, expr_node,
+                                               link);
 
-            if (expr->oper == ast_aggregate)
-               _mesa_ast_set_aggregate_type(type, expr, state);
+         if (expr->oper == ast_aggregate) {
+            _mesa_ast_set_aggregate_type(type->fields.structure[i].type, expr);
          }
       }
-   } else {
-      /* If the aggregate is a matrix, set its columns' types. */
-      const char *name;
-      const glsl_type *const constructor_type =
-         ai->constructor_type->glsl_type(&name, state);
-
-      if (constructor_type->is_matrix()) {
-         for (exec_node *expr_node = ai->expressions.head;
-              !expr_node->is_tail_sentinel();
-              expr_node = expr_node->next) {
-            ast_expression *expr = exec_node_data(ast_expression, expr_node,
-                                                  link);
-
-            /* Declaration shadows the <type> parameter. */
-            ast_type_specifier *type = new(ctx)
-               ast_type_specifier(_mesa_ast_get_matrix_column_type_name(name));
-
-            if (expr->oper == ast_aggregate)
-               _mesa_ast_set_aggregate_type(type, expr, state);
-         }
+   /* If the aggregate is a matrix, set its columns' types. */
+   } else if (type->is_matrix()) {
+      for (exec_node *expr_node = ai->expressions.head;
+           !expr_node->is_tail_sentinel();
+           expr_node = expr_node->next) {
+         ast_expression *expr = exec_node_data(ast_expression, expr_node,
+                                               link);
+
+         if (expr->oper == ast_aggregate)
+            _mesa_ast_set_aggregate_type(type->column_type(), expr);
       }
    }
 }