glsl: Fix interstage uniform interface block link error detection.
authorPaul Berry <stereotype441@gmail.com>
Fri, 15 Nov 2013 22:23:45 +0000 (14:23 -0800)
committerPaul Berry <stereotype441@gmail.com>
Thu, 21 Nov 2013 23:05:09 +0000 (15:05 -0800)
Previously, we checked for interstage uniform interface block link
errors in validate_interstage_interface_blocks(), which is only called
on pairs of adjacent shader stages.  Therefore, we failed to detect
uniform interface block mismatches between non-adjacent shader stages.

Before the introduction of geometry shaders, this wasn't a problem,
because the only supported shader stages were vertex and fragment
shaders, therefore they were always adjacent.  However, now that we
allow a program to contain vertex, geometry, and fragment shaders,
that is no longer the case.

Fixes piglit test "skip-stage-uniform-block-array-size-mismatch".

Cc: "10.0" <mesa-stable@lists.freedesktop.org>
v2: Rename validate_interstage_interface_blocks() to
validate_interstage_inout_blocks() to reflect the fact that it no
longer validates uniform blocks.

Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
v3: Make validate_interstage_inout_blocks() skip uniform blocks.

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
src/glsl/link_interface_blocks.cpp
src/glsl/linker.cpp
src/glsl/linker.h

index 528d4a1..6900fa9 100644 (file)
@@ -308,57 +308,78 @@ validate_intrastage_interface_blocks(struct gl_shader_program *prog,
 }
 
 void
-validate_interstage_interface_blocks(struct gl_shader_program *prog,
-                                     const gl_shader *producer,
-                                     const gl_shader *consumer)
+validate_interstage_inout_blocks(struct gl_shader_program *prog,
+                                 const gl_shader *producer,
+                                 const gl_shader *consumer)
 {
-   interface_block_definitions inout_interfaces;
-   interface_block_definitions uniform_interfaces;
+   interface_block_definitions definitions;
    const bool extra_array_level = consumer->Type == GL_GEOMETRY_SHADER;
 
-   /* Add non-output interfaces from the consumer to the symbol table. */
+   /* Add input interfaces from the consumer to the symbol table. */
    foreach_list(node, consumer->ir) {
       ir_variable *var = ((ir_instruction *) node)->as_variable();
-      if (!var || !var->get_interface_type() || var->mode == ir_var_shader_out)
+      if (!var || !var->get_interface_type() || var->mode != ir_var_shader_in)
          continue;
 
-      interface_block_definitions *definitions = var->mode == ir_var_uniform ?
-         &uniform_interfaces : &inout_interfaces;
-      definitions->store(interface_block_definition(var));
+      definitions.store(interface_block_definition(var));
    }
 
-   /* Verify that the producer's interfaces match. */
+   /* Verify that the producer's output interfaces match. */
    foreach_list(node, producer->ir) {
       ir_variable *var = ((ir_instruction *) node)->as_variable();
-      if (!var || !var->get_interface_type() || var->mode == ir_var_shader_in)
+      if (!var || !var->get_interface_type() || var->mode != ir_var_shader_out)
          continue;
 
-      interface_block_definitions *definitions = var->mode == ir_var_uniform ?
-         &uniform_interfaces : &inout_interfaces;
       interface_block_definition *consumer_def =
-         definitions->lookup(var->get_interface_type()->name);
+         definitions.lookup(var->get_interface_type()->name);
 
       /* The consumer doesn't use this output block.  Ignore it. */
       if (consumer_def == NULL)
          continue;
 
       const interface_block_definition producer_def(var);
-      bool match;
-      if (var->mode == ir_var_uniform) {
-         /* Uniform matching rules are the same for interstage and intrastage
-          * linking.
-          */
-         match = intrastage_match(consumer_def, &producer_def,
-                                  (ir_variable_mode) var->mode);
-      } else {
-         match = interstage_match(&producer_def, consumer_def,
-                                  extra_array_level);
-      }
 
-      if (!match) {
+      if (!interstage_match(&producer_def, consumer_def, extra_array_level)) {
          linker_error(prog, "definitions of interface block `%s' do not "
                       "match\n", var->get_interface_type()->name);
          return;
       }
    }
 }
+
+
+void
+validate_interstage_uniform_blocks(struct gl_shader_program *prog,
+                                   gl_shader **stages, int num_stages)
+{
+   interface_block_definitions definitions;
+
+   for (int i = 0; i < num_stages; i++) {
+      if (stages[i] == NULL)
+         continue;
+
+      const gl_shader *stage = stages[i];
+      foreach_list(node, stage->ir) {
+         ir_variable *var = ((ir_instruction *) node)->as_variable();
+         if (!var || !var->get_interface_type() || var->mode != ir_var_uniform)
+            continue;
+
+         interface_block_definition *old_def =
+            definitions.lookup(var->get_interface_type()->name);
+         const interface_block_definition new_def(var);
+         if (old_def == NULL) {
+            definitions.store(new_def);
+         } else {
+            /* Interstage uniform matching rules are the same as intrastage
+             * uniform matchin rules (for uniforms, it is as though all
+             * shaders are in the same shader stage).
+             */
+            if (!intrastage_match(old_def, &new_def, ir_var_uniform)) {
+               linker_error(prog, "definitions of interface block `%s' do not "
+                            "match\n", var->get_interface_type()->name);
+               return;
+            }
+         }
+      }
+   }
+}
index 1d53b65..fac186a 100644 (file)
@@ -2154,8 +2154,8 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
       if (prog->_LinkedShaders[i] == NULL)
          continue;
 
-      validate_interstage_interface_blocks(prog, prog->_LinkedShaders[prev],
-                                           prog->_LinkedShaders[i]);
+      validate_interstage_inout_blocks(prog, prog->_LinkedShaders[prev],
+                                       prog->_LinkedShaders[i]);
       if (!prog->LinkStatus)
          goto done;
 
@@ -2168,6 +2168,11 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
       prev = i;
    }
 
+   /* Cross-validate uniform blocks between shader stages */
+   validate_interstage_uniform_blocks(prog, prog->_LinkedShaders,
+                                      MESA_SHADER_TYPES);
+   if (!prog->LinkStatus)
+      goto done;
 
    for (unsigned int i = 0; i < MESA_SHADER_TYPES; i++) {
       if (prog->_LinkedShaders[i] != NULL)
index 7b1f6f9..130915d 100644 (file)
@@ -65,9 +65,13 @@ validate_intrastage_interface_blocks(struct gl_shader_program *prog,
                                      unsigned num_shaders);
 
 void
-validate_interstage_interface_blocks(struct gl_shader_program *prog,
-                                     const gl_shader *producer,
-                                     const gl_shader *consumer);
+validate_interstage_inout_blocks(struct gl_shader_program *prog,
+                                 const gl_shader *producer,
+                                 const gl_shader *consumer);
+
+void
+validate_interstage_uniform_blocks(struct gl_shader_program *prog,
+                                   gl_shader **stages, int num_stages);
 
 extern void
 link_assign_atomic_counter_resources(struct gl_context *ctx,