glsl: fix subroutine mismatch between declarations/definitions

Previously, when q.subroutine was set to 1, a new subroutine
declaration was added to the AST, while 0 meant a subroutine
definition has been detected by the parser.

Thus, setting the q.subroutine flag in both situations is
obviously wrong because a new type identifier is added instead
of trying to match the declaration. To fix it up, introduce
ast_type_qualifier::is_subroutine_decl() to differentiate
declarations and definitions easily.

This fixes a regression with:
arb_shader_subroutine/compiler/direct-call.vert

Cc: Mark Janes <[email protected]>
Fixes: be8aa76afd ("glsl: remove unecessary flags.q.subroutine_def")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100026
Signed-off-by: Samuel Pitoiset <[email protected]>
Reviewed-by: Timothy Arceri <[email protected]>

5 files changed, 18 insertions, 8 deletions

diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
index d27b9407440..55cc5df8f31 100644
--- a/src/compiler/glsl/ast.h
+++ b/src/compiler/glsl/ast.h
@@ -770,6 +770,11 @@ struct ast_type_qualifier {
     */
    bool has_memory() const;
 
+   /**
+    * Return true if the qualifier is a subroutine declaration.
+    */
+   bool is_subroutine_decl() const;
+
    bool merge_qualifier(YYLTYPE *loc,
 			_mesa_glsl_parse_state *state,
                         const ast_type_qualifier &q,
diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index a90813033fb..59d03c9c962 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3256,7 +3256,7 @@ apply_explicit_location(const struct ast_type_qualifier *qual,
       }
 
       /* Check if index was set for the uniform instead of the function */
-      if (qual->flags.q.explicit_index && qual->flags.q.subroutine) {
+      if (qual->flags.q.explicit_index && qual->is_subroutine_decl()) {
          _mesa_glsl_error(loc, state, "an index qualifier can only be "
                           "used with subroutine functions");
          return;
@@ -3742,7 +3742,7 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
       }
    }
 
-   if (qual->flags.q.subroutine && !qual->flags.q.uniform) {
+   if (qual->is_subroutine_decl() && !qual->flags.q.uniform) {
       _mesa_glsl_error(loc, state,
                        "`subroutine' may only be applied to uniforms, "
                        "subroutine type declarations, or function definitions");
@@ -4780,7 +4780,7 @@ ast_declarator_list::hir(exec_list *instructions,
          continue;
       }
 
-      if (this->type->qualifier.flags.q.subroutine) {
+      if (this->type->qualifier.is_subroutine_decl()) {
          const glsl_type *t;
          const char *name;
 
@@ -4879,7 +4879,7 @@ ast_declarator_list::hir(exec_list *instructions,
           */
          if (this->type->qualifier.flags.q.attribute) {
             mode = "attribute";
-         } else if (this->type->qualifier.flags.q.subroutine) {
+         } else if (this->type->qualifier.is_subroutine_decl()) {
             mode = "subroutine uniform";
          } else if (this->type->qualifier.flags.q.uniform) {
             mode = "uniform";
@@ -5629,7 +5629,7 @@ ast_function::hir(exec_list *instructions,
    f = state->symbols->get_function(name);
    if (f == NULL) {
       f = new(ctx) ir_function(name);
-      if (!this->return_type->qualifier.flags.q.subroutine) {
+      if (!this->return_type->qualifier.is_subroutine_decl()) {
          if (!state->symbols->add_function(f)) {
             /* This function name shadows a non-function use of the same name. */
             YYLTYPE loc = this->get_location();
@@ -5787,7 +5787,7 @@ ast_function::hir(exec_list *instructions,
 
    }
 
-   if (this->return_type->qualifier.flags.q.subroutine) {
+   if (this->return_type->qualifier.is_subroutine_decl()) {
       if (!state->symbols->add_type(this->identifier, glsl_type::get_subroutine_instance(this->identifier))) {
          _mesa_glsl_error(& loc, state, "type '%s' previously defined", this->identifier);
          return NULL;
diff --git a/src/compiler/glsl/ast_type.cpp b/src/compiler/glsl/ast_type.cpp
index 5f868a81f21..d302fc45fd7 100644
--- a/src/compiler/glsl/ast_type.cpp
+++ b/src/compiler/glsl/ast_type.cpp
@@ -112,6 +112,11 @@ bool ast_type_qualifier::has_memory() const
           || this->flags.q.write_only;
 }
 
+bool ast_type_qualifier::is_subroutine_decl() const
+{
+   return this->flags.q.subroutine && !this->subroutine_list;
+}
+
 static bool
 validate_prim_type(YYLTYPE *loc,
                    _mesa_glsl_parse_state *state,
diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
index 59453d72f64..e703073c9ca 100644
--- a/src/compiler/glsl/glsl_parser.yy
+++ b/src/compiler/glsl/glsl_parser.yy
@@ -900,7 +900,7 @@ function_header:
       $$->return_type = $1;
       $$->identifier = $2;
 
-      if ($1->qualifier.flags.q.subroutine) {
+      if ($1->qualifier.is_subroutine_decl()) {
          /* add type for IDENTIFIER search */
          state->symbols->add_type($2, glsl_type::get_subroutine_instance($2));
       } else
diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
index e88dd071b3a..44fb46ab838 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -1072,7 +1072,7 @@ _mesa_ast_process_interface_block(YYLTYPE *locp,
 void
 _mesa_ast_type_qualifier_print(const struct ast_type_qualifier *q)
 {
-   if (q->flags.q.subroutine)
+   if (q->is_subroutine_decl())
       printf("subroutine ");
 
    if (q->subroutine_list) {