[Mesa-dev] [PATCH] glsl: Simplify aggregate type inference to prepare for ARB_arrays_of_arrays.
Paul Berry
stereotype441 at gmail.com
Tue Jan 21 16:14:31 PST 2014
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.
---
Timothy: I was inspired to write this patch by the complexities you
encountered during "[PATCH V2 5/8] glsl: Aggregate initializer support
for arrays of array". Can you try rebasing your series on top of this
patch to see if it simplifies things? I believe that with these
changes, you should be able to drop patch 5/8 entirely.
src/glsl/ast.h | 16 +++--
src/glsl/ast_function.cpp | 8 +--
src/glsl/ast_to_hir.cpp | 7 +++
src/glsl/glsl_parser.yy | 27 --------
src/glsl/glsl_parser_extras.cpp | 136 +++++++---------------------------------
5 files changed, 46 insertions(+), 148 deletions(-)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 76911f0..b24052b 100644
--- 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 2d05d07..4c5b0e4 100644
--- 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 4cc8eb1..8d13610 100644
--- 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 1c56d6f..5451b76 100644
--- 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 ddb3d2d..ead5d6e 100644
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -635,25 +635,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
* (process_array_constructor, et al) sufficient information to do type
@@ -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);
}
}
}
--
1.8.5.3
More information about the mesa-dev
mailing list