[Mesa-dev] [PATCH 2/9] glsl: do not lookup struct types by typename

[Timothy Arceri]

On 16/05/17 09: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.

With the above justified or removed, and the commit message updated in 
patch 6, 2-9 are:

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>


> 
>> +      } 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