[Mesa-dev] [PATCH v2] glsl: Prohibit illegal mixing of redeclarations inside/outside gl_PerVertex.
Ian Romanick
idr at freedesktop.org
Wed Nov 20 11:26:36 PST 2013
On 11/19/2013 05:55 PM, Paul Berry wrote:
> 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 a boolean to ir_variable to
But it's not a boolean. :) Tri-state?
> 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" <mesa-stable at lists.freedesktop.org>
>
> v2: Don't set "how_declared" redundantly in builtin_variables.cpp.
> Properly clone "how_declared".
> ---
> src/glsl/ast_to_hir.cpp | 20 ++++++++++++++++++++
> src/glsl/builtin_variables.cpp | 1 +
> src/glsl/ir.cpp | 3 ++-
> src/glsl/ir.h | 36 ++++++++++++++++++++++++++++++++++++
> src/glsl/ir_clone.cpp | 1 +
> 5 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 76b256c..0128047 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;
Is this correct? At this point redeclaring_per_vertex is true, so it
seems like this should be ir_var_declared_in_block.
> 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 4d44104..d57324c 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 1b49736..ffff297 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 7859702..4f775da 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -294,6 +294,34 @@ enum ir_variable_mode {
> };
>
> /**
> + * 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.
> *
> * The AMD/ARB_conservative_depth extensions allow gl_FragDepth to be redeclared
> @@ -526,6 +554,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.
> *
> * \sa ir_variable_mode
> diff --git a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp
> index b0f173a..40ed33a 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;
>
More information about the mesa-dev
mailing list