From: Paul Berry Date: Thu, 14 Nov 2013 00:53:18 +0000 (-0800) Subject: glsl: Prohibit illegal mixing of redeclarations inside/outside gl_PerVertex. X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=2bbcf19acad530d339ffe8e007fe2f6a244e1580;p=mesa.git glsl: Prohibit illegal mixing of redeclarations inside/outside gl_PerVertex. From section 7.1 (Built-In Language Variables) of the GLSL 4.10 spec: Also, if a built-in interface block is redeclared, no member of the built-in declaration can be redeclared outside the block redeclaration. We have been regarding this text as a clarification to the behaviour established for gl_PerVertex by GLSL 1.50, so we apply it regardless of GLSL version. This patch enforces the rule by adding an enum to ir_variable to track how the variable was declared: implicitly, normally, or in an interface block. Fixes piglit tests: - gs-redeclares-pervertex-out-after-global-redeclaration.geom - vs-redeclares-pervertex-out-after-global-redeclaration.vert - gs-redeclares-pervertex-out-after-other-global-redeclaration.geom - vs-redeclares-pervertex-out-after-other-global-redeclaration.vert - gs-redeclares-pervertex-out-before-global-redeclaration - vs-redeclares-pervertex-out-before-global-redeclaration Cc: "10.0" v2: Don't set "how_declared" redundantly in builtin_variables.cpp. Properly clone "how_declared". Reviewed-by: Ian Romanick --- diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 76b256c731a..01280478c95 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -3355,6 +3355,15 @@ ast_declarator_list::hir(exec_list *instructions, ir_variable *earlier = get_variable_being_redeclared(var, decl->get_location(), state, false /* allow_all_redeclarations */); + if (earlier != NULL) { + if (strncmp(var->name, "gl_", 3) == 0 && + earlier->how_declared == ir_var_declared_in_block) { + _mesa_glsl_error(&loc, state, + "`%s' has already been redeclared using " + "gl_PerVertex", var->name); + } + earlier->how_declared = ir_var_declared_normally; + } if (decl->initializer != NULL) { result = process_initializer((earlier == NULL) ? var : earlier, @@ -5048,6 +5057,7 @@ ast_interface_block::hir(exec_list *instructions, _mesa_glsl_error(&loc, state, "`%s' redeclared", this->instance_name); } + earlier->how_declared = ir_var_declared_normally; earlier->type = var->type; earlier->reinit_interface_type(block_type); delete var; @@ -5078,7 +5088,11 @@ ast_interface_block::hir(exec_list *instructions, _mesa_glsl_error(&loc, state, "redeclaration of gl_PerVertex can only " "include built-in variables"); + } else if (earlier->how_declared == ir_var_declared_normally) { + _mesa_glsl_error(&loc, state, + "`%s' has already been redeclared", var->name); } else { + earlier->how_declared = ir_var_declared_in_block; earlier->reinit_interface_type(block_type); } continue; @@ -5125,6 +5139,12 @@ ast_interface_block::hir(exec_list *instructions, if (var != NULL && var->get_interface_type() == earlier_per_vertex && var->mode == var_mode) { + if (var->how_declared == ir_var_declared_normally) { + _mesa_glsl_error(&loc, state, + "redeclaration of gl_PerVertex cannot " + "follow a redeclaration of `%s'", + var->name); + } state->symbols->disable_variable(var->name); var->remove(); } diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp index 4d441045a83..d57324c2fba 100644 --- a/src/glsl/builtin_variables.cpp +++ b/src/glsl/builtin_variables.cpp @@ -434,6 +434,7 @@ builtin_variable_generator::add_variable(const char *name, enum ir_variable_mode mode, int slot) { ir_variable *var = new(symtab) ir_variable(type, name, mode); + var->how_declared = ir_var_declared_implicitly; switch (var->mode) { case ir_var_auto: diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp index 1b4973612e0..ffff2976aa4 100644 --- a/src/glsl/ir.cpp +++ b/src/glsl/ir.cpp @@ -1586,7 +1586,8 @@ ir_variable::ir_variable(const struct glsl_type *type, const char *name, ir_variable_mode mode) : max_array_access(0), max_ifc_array_access(NULL), read_only(false), centroid(false), invariant(false), - mode(mode), interpolation(INTERP_QUALIFIER_NONE), atomic() + how_declared(ir_var_declared_normally), mode(mode), + interpolation(INTERP_QUALIFIER_NONE), atomic() { this->ir_type = ir_type_variable; this->type = type; diff --git a/src/glsl/ir.h b/src/glsl/ir.h index 7859702ed01..4f775da4bd1 100644 --- a/src/glsl/ir.h +++ b/src/glsl/ir.h @@ -293,6 +293,34 @@ enum ir_variable_mode { ir_var_mode_count /**< Number of variable modes */ }; +/** + * Enum keeping track of how a variable was declared. For error checking of + * the gl_PerVertex redeclaration rules. + */ +enum ir_var_declaration_type { + /** + * Normal declaration (for most variables, this means an explicit + * declaration. Exception: temporaries are always implicitly declared, but + * they still use ir_var_declared_normally). + * + * Note: an ir_variable that represents a named interface block uses + * ir_var_declared_normally. + */ + ir_var_declared_normally = 0, + + /** + * Variable was explicitly declared (or re-declared) in an unnamed + * interface block. + */ + ir_var_declared_in_block, + + /** + * Variable is an implicitly declared built-in that has not been explicitly + * re-declared by the shader. + */ + ir_var_declared_implicitly, +}; + /** * \brief Layout qualifiers for gl_FragDepth. * @@ -525,6 +553,14 @@ public: */ unsigned assigned:1; + /** + * Enum indicating how the variable was declared. See + * ir_var_declaration_type. + * + * This is used to detect certain kinds of illegal variable redeclarations. + */ + unsigned how_declared:2; + /** * Storage class of the variable. * diff --git a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp index b0f173a62e6..40ed33afcee 100644 --- a/src/glsl/ir_clone.cpp +++ b/src/glsl/ir_clone.cpp @@ -68,6 +68,7 @@ ir_variable::clone(void *mem_ctx, struct hash_table *ht) const var->has_initializer = this->has_initializer; var->depth_layout = this->depth_layout; var->assigned = this->assigned; + var->how_declared = this->how_declared; var->used = this->used; var->num_state_slots = this->num_state_slots;