From 9fc86d4f53a66a147f38f54b09629372bd085658 Mon Sep 17 00:00:00 2001 From: Samuel Pitoiset Date: Wed, 1 Mar 2017 22:09:28 +0100 Subject: [PATCH] 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 Fixes: be8aa76afd ("glsl: remove unecessary flags.q.subroutine_def") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100026 Signed-off-by: Samuel Pitoiset Reviewed-by: Timothy Arceri --- src/compiler/glsl/ast.h | 5 +++++ src/compiler/glsl/ast_to_hir.cpp | 12 ++++++------ src/compiler/glsl/ast_type.cpp | 5 +++++ src/compiler/glsl/glsl_parser.yy | 2 +- src/compiler/glsl/glsl_parser_extras.cpp | 2 +- 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) { -- 2.30.2