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

[Nicolai Hähnle]

On 16.05.2017 01:24, Timothy Arceri wrote:
On 15/05/17 19:27, Nicolai Hähnle wrote:
From: Nicolai Hähnle <nicolai.haeh...@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev