[Mesa-dev] [PATCH 14/14] glsl: Add support for C-style initializers.
Matt Turner
mattst88 at gmail.com
Thu Jul 11 20:52:59 PDT 2013
On Thu, Jul 11, 2013 at 4:10 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 07/03/2013 01:59 PM, Matt Turner wrote:
>>
>> Required by GL_ARB_shading_language_420pack.
>>
>> Parts based on work done by Todd Previte and Ken Graunke, implementing
>> basic support for C-style initializers of arrays.
>> ---
>> src/glsl/ast.h | 5 ++
>> src/glsl/ast_to_hir.cpp | 14 +++-
>> src/glsl/glsl_parser.yy | 57 +++++++++++++
>> src/glsl/glsl_parser_extras.cpp | 179
>> ++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 254 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
>> index ba14382..f4660ea 100644
>> --- a/src/glsl/ast.h
>> +++ b/src/glsl/ast.h
>> @@ -915,6 +915,11 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,
>> ir_rvalue *array, ir_rvalue *idx,
>> 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);
>> +
>> void
>> emit_function(_mesa_glsl_parse_state *state, ir_function *f);
>>
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index 8d78f34..b9f8df5 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -4005,7 +4005,19 @@ ast_type_specifier::hir(exec_list *instructions,
>> return NULL;
>> }
>>
>> - if (this->structure != NULL)
>> + /* _mesa_ast_set_aggregate_type() sets the <structure> field so that
>> + * process_record_constructor() can do type-checking on C-style
>> initializer
>> + * expressions of structs, but ast_struct_specifier should only be
>> translated
>> + * to HIR if it is declaring the type of a structure.
>> + *
>> + * The ->is_declaration field is false for initializers of variables
>> + * declared separately from the struct's type definition.
>> + *
>> + * struct S { ... }; (is_declaration = true)
>> + * struct T { ... } t = { ... }; (is_declaration = true)
>> + * S s = { ... }; (is_declaration = false)
>> + */
>> + if (this->structure != NULL && this->structure->is_declaration)
>> return this->structure->hir(instructions, state);
>>
>> return NULL;
>> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
>> index 56367f8..c8c660f 100644
>> --- a/src/glsl/glsl_parser.yy
>> +++ b/src/glsl/glsl_parser.yy
>> @@ -221,6 +221,7 @@ static void yyerror(YYLTYPE *loc,
>> _mesa_glsl_parse_state *st, const char *msg)
>> %type <declarator_list> init_declarator_list
>> %type <declarator_list> single_declaration
>> %type <expression> initializer
>> +%type <expression> initializer_list
>> %type <node> declaration
>> %type <node> declaration_statement
>> %type <node> jump_statement
>> @@ -961,6 +962,12 @@ 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;
>
>
> Some trivial formatting flubs here. { in the wrong place, no space before
> the *. Similar cases below. I assume this is Todd's code? :)
Indeed. :)
>> + 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
>> {
>> @@ -971,6 +978,12 @@ 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
>> {
>> @@ -981,6 +994,11 @@ 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);
>> + }
>> }
>> ;
>>
>> @@ -1028,6 +1046,12 @@ 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
>> {
>> @@ -1037,6 +1061,12 @@ 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
>> {
>> @@ -1046,6 +1076,10 @@ 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.
>> {
>> @@ -1503,6 +1537,7 @@ struct_specifier:
>> $$ = new(ctx) ast_struct_specifier($2, $4);
>> $$->set_location(yylloc);
>> state->symbols->add_type($2, glsl_type::void_type);
>> + state->symbols->add_type_ast($2, new(ctx)
>> ast_type_specifier($$));
>> }
>> | STRUCT '{' struct_declaration_list '}'
>> {
>> @@ -1570,6 +1605,28 @@ struct_declarator:
>>
>> initializer:
>> assignment_expression
>> + | '{' initializer_list '}'
>> + {
>> + $$ = $2;
>> + }
>> + | '{' initializer_list ',' '}'
>> + {
>> + $$ = $2;
>> + }
>> + ;
>> +
>> +initializer_list:
>> + initializer
>> + {
>> + void *ctx = state;
>> + $$ = new(ctx) ast_aggregate_initializer();
>> + $$->set_location(yylloc);
>> + $$->expressions.push_tail(& $1->link);
>> + }
>> + | initializer_list ',' initializer
>> + {
>> + $1->expressions.push_tail(& $3->link);
>> + }
>
>
> I believe that all this grammar is correct, but do we have tests for
> spurious curly braces? Things like
>
> vec4 v = {{{1, 2, 3, 4}}};
No, I'll write one.
>> ;
>>
>> declaration_statement:
>> diff --git a/src/glsl/glsl_parser_extras.cpp
>> b/src/glsl/glsl_parser_extras.cpp
>> index bab81fe..59844dd 100644
>> --- a/src/glsl/glsl_parser_extras.cpp
>> +++ b/src/glsl/glsl_parser_extras.cpp
>> @@ -663,6 +663,185 @@ _mesa_glsl_process_extension(const char *name,
>> YYLTYPE *name_locp,
>> return true;
>> }
>>
>> +
>> +/**
>> + * 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];
>
>
> I can already hear static analysis tools getting angry about this code... :)
I could add asserts, but as we've seen, static analysis ignores
typically ignores them.
>> +}
>> +
>> +/**
>> + * 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
>> + * checking.
>> + *
>> + * Operates on assignments involving an aggregate initializer. E.g.,
>> + *
>> + * vec4 pos = {1.0, -1.0, 0.0, 1.0};
>> + *
>> + * or more ridiculously,
>> + *
>> + * struct S {
>> + * vec4 v[2];
>> + * };
>> + *
>> + * struct {
>> + * S a[2], b;
>> + * int c;
>> + * } aggregate = {
>> + * {
>> + * {
>> + * {
>> + * {1.0, 2.0, 3.0, 4.0}, // a[0].v[0]
>> + * {5.0, 6.0, 7.0, 8.0} // a[0].v[1]
>> + * } // a[0].v
>> + * }, // a[0]
>> + * {
>> + * {
>> + * {1.0, 2.0, 3.0, 4.0}, // a[1].v[0]
>> + * {5.0, 6.0, 7.0, 8.0} // a[1].v[1]
>> + * } // a[1].v
>> + * } // a[1]
>> + * }, // a
>> + * {
>> + * {
>> + * {1.0, 2.0, 3.0, 4.0}, // b.v[0]
>> + * {5.0, 6.0, 7.0, 8.0} // b.v[1]
>> + * } // b.v
>> + * }, // b
>> + * 4 // c
>> + * };
>> + *
>> + * This pass is necessary because the right-hand side of <type> e = { ...
>> }
>> + * 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)
>> +{
>> + 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;
>> + }
>> +
>> + /* 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.
>> + *
>> + * XXX: Update when ARB_array_of_arrays is supported.
>
>
> Use FINISHME instead of XXX.
>
>
>> + */
>> + 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) {
>> + 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 the aggregate is a struct, recursively set its fields' types. */
>> + } else if (ai->constructor_type->structure) {
>> + ai->constructor_type->structure->is_declaration = is_declaration;
>> + 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;
>> +
>> + if (!decl_list->type->specifier->is_array) {
>> + /* XXX: Update when ARB_array_of_arrays is supported. */
>
>
> Use FINISHME instead of XXX.
>
> Also, this could use a comment. I had to really think about it to
> understand what was going on.
Sure, I've updated it with
/* 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.
*/
Thanks for the review!
More information about the mesa-dev
mailing list