[Mesa-dev] [PATCH 2/9] glsl: do not lookup struct types by typename
Nicolai Hähnle
nhaehnle at gmail.com
Mon May 15 23:42:49 UTC 2017
On 16.05.2017 01:24, Timothy Arceri wrote:
> On 15/05/17 19:27, Nicolai Hähnle wrote:
>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>
>> This changes the logic during the conversion of the declaration list
>>
>> struct S {
>> ...
>> } v;
>>
>> from AST to IR, but should not change the end result.
>>
>> When assigning the type of v, instead of looking `S' up in the symbol
>> table, we read the type from the member variable of ast_struct_specifier.
>>
>> This change is necessary for the subsequent change to how anonymous types
>> are handled.
>> ---
>> src/compiler/glsl/ast.h | 1 +
>> src/compiler/glsl/ast_to_hir.cpp | 17 ++++++++++-------
>> src/compiler/glsl/glsl_parser_extras.cpp | 1 +
>> 3 files changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
>> index 9327e03..3bf4b08 100644
>> --- a/src/compiler/glsl/ast.h
>> +++ b/src/compiler/glsl/ast.h
>> @@ -837,20 +837,21 @@ public:
>> virtual void print(void) const;
>> virtual ir_rvalue *hir(exec_list *instructions,
>> struct _mesa_glsl_parse_state *state);
>> const char *name;
>> ast_type_qualifier *layout;
>> /* List of ast_declarator_list * */
>> exec_list declarations;
>> bool is_declaration;
>> + const glsl_type *type;
>> };
>> class ast_type_specifier : public ast_node {
>> public:
>> /** Construct a type specifier from a type name */
>> ast_type_specifier(const char *name)
>> : type_name(name), structure(NULL), array_specifier(NULL),
>> default_precision(ast_precision_none)
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp
>> b/src/compiler/glsl/ast_to_hir.cpp
>> index 0dc69ef..2221c5f 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -2352,21 +2352,24 @@ precision_qualifier_allowed(const glsl_type
>> *type)
>> return (t->is_float() || t->is_integer() || t->contains_opaque()) &&
>> !t->is_record();
>> }
>> const glsl_type *
>> ast_type_specifier::glsl_type(const char **name,
>> struct _mesa_glsl_parse_state *state)
>> const
>> {
>> const struct glsl_type *type;
>> - type = state->symbols->get_type(this->type_name);
>> + if (structure)
>> + type = structure->type;
>> + else
>> + type = state->symbols->get_type(this->type_name);
>> *name = this->type_name;
>> YYLTYPE loc = this->get_location();
>> type = process_array_type(&loc, type, this->array_specifier, state);
>> return type;
>> }
>> /**
>> * From the OpenGL ES 3.0 spec, 4.5.4 Default Precision Qualifiers:
>> @@ -7474,36 +7477,36 @@ ast_struct_specifier::hir(exec_list
>> *instructions,
>> ir_var_auto,
>> layout,
>> 0, /* for interface
>> only */
>> 0, /* for interface
>> only */
>> 0, /* for interface
>> only */
>> expl_location,
>> 0 /* for interface
>> only */);
>> validate_identifier(this->name, loc, state);
>> - const glsl_type *t =
>> - glsl_type::get_record_instance(fields, decl_count, this->name);
>> + type = glsl_type::get_record_instance(fields, decl_count,
>> this->name);
>> - if (!state->symbols->add_type(name, t)) {
>> + if (!state->symbols->add_type(name, type)) {
>> const glsl_type *match = state->symbols->get_type(name);
>> /* allow struct matching for desktop GL - older UE4 does this */
>> - if (match != NULL && state->is_version(130, 0) &&
>> match->record_compare(t, false))
>> + if (match != NULL && state->is_version(130, 0) &&
>> match->record_compare(type, false)) {
>> _mesa_glsl_warning(& loc, state, "struct `%s' previously
>> defined", name);
>> - else
>> + type = match;
>
> I'm not following why we need to do this.
In the warning case, `type' is basically discarded. The symbol table is
left unmodified, so looking up the type name yields the old glsl_type
instance (aka `match'), and so we should also return match for the
current ast_struct_specifier. I hope that clears things up?
Cheers,
Nicolai
>
>> + } else
>> _mesa_glsl_error(& loc, state, "struct `%s' previously
>> defined", name);
>> } else {
>> const glsl_type **s = reralloc(state, state->user_structures,
>> const glsl_type *,
>> state->num_user_structures + 1);
>> if (s != NULL) {
>> - s[state->num_user_structures] = t;
>> + s[state->num_user_structures] = type;
>> state->user_structures = s;
>> state->num_user_structures++;
>> }
>> }
>> /* Structure type definitions do not have r-values.
>> */
>> return NULL;
>> }
>> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp
>> b/src/compiler/glsl/glsl_parser_extras.cpp
>> index d731e35..53ae95d 100644
>> --- a/src/compiler/glsl/glsl_parser_extras.cpp
>> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
>> @@ -1681,20 +1681,21 @@
>> ast_struct_specifier::ast_struct_specifier(void *lin_ctx, const char
>> *identifier
>> mtx_lock(&mutex);
>> count = anon_count++;
>> mtx_unlock(&mutex);
>> identifier = linear_asprintf(lin_ctx, "#anon_struct_%04x",
>> count);
>> }
>> name = identifier;
>>
>> this->declarations.push_degenerate_list_at_head(&declarator_list->link);
>> is_declaration = true;
>> layout = NULL;
>> + type = NULL;
>> }
>> void ast_subroutine_list::print(void) const
>> {
>> foreach_list_typed (ast_node, ast, link, & this->declarations) {
>> if (&ast->link != this->declarations.get_head())
>> printf(", ");
>> ast->print();
>> }
>> }
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list