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" <mesa-stable@lists.freedesktop.org>
v2: Don't set "how_declared" redundantly in builtin_variables.cpp.
Properly clone "how_declared".

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 76b256c731ad9b9ac4aab70563ce6c208e89e72d..01280478c95714033beff01708c5b8f462afded6 100644 (file)
--- 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 4d441045a8310c3554b400c55b9c30b0aa38a0ba..d57324c2fba8481cc197cfeae8a661608c1a10cd 100644 (file)
--- 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 1b4973612e02d82ded52de1742a88ac9b7a08aa2..ffff2976aa4aaf90502a8930b71be9a3196de3b5 100644 (file)
--- 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 7859702ed015c7d2d2464048da2500d0f36030bb..4f775da4bd17314e87a54601f6238cd0b9a1553e 100644 (file)
--- 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 b0f173a62e6c1b3a063bff66f62a32cf1d2a4109..40ed33afcee85d37ebf52fa4e07e3a12c9c26131 100644 (file)
--- 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;