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

[Timothy Arceri]

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.

> +      } 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();
>      }
>   }
>