This patch regresses:
piglit.spec.arb_shader_subroutine.linker.no-overloads_vert
The program in the test should fail to link, but does not.
Samuel Pitoiset <samuel.pitoi...@gmail.com> writes:
> 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 <mark.a.ja...@intel.com>
> Fixes: be8aa76afd ("glsl: remove unecessary flags.q.subroutine_def")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100026
> Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>
> ---
> 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 d27b940744..55cc5df8f3 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 a90813033f..59d03c9c96 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 5f868a81f2..d302fc45fd 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 59453d72f6..e703073c9c 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 e88dd071b3..44fb46ab83 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.12.0
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
