[Mesa-dev] [PATCH] glsl: Fix interstage uniform interface block link error detection.

Paul Berry stereotype441 at gmail.com
Fri Nov 15 14:56:59 PST 2013


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 at lists.freedesktop.org>
---
 src/glsl/link_interface_blocks.cpp | 61 +++++++++++++++++++++++++-------------
 src/glsl/linker.cpp                |  5 ++++
 src/glsl/linker.h                  |  4 +++
 3 files changed, 50 insertions(+), 20 deletions(-)

diff --git a/src/glsl/link_interface_blocks.cpp b/src/glsl/link_interface_blocks.cpp
index a7fceb9..31269f6 100644
--- a/src/glsl/link_interface_blocks.cpp
+++ b/src/glsl/link_interface_blocks.cpp
@@ -293,8 +293,7 @@ validate_interstage_interface_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. */
@@ -303,9 +302,7 @@ validate_interstage_interface_blocks(struct gl_shader_program *prog,
       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;
-      definitions->store(interface_block_definition(var));
+      definitions.store(interface_block_definition(var));
    }
 
    /* Verify that the producer's interfaces match. */
@@ -314,32 +311,56 @@ validate_interstage_interface_blocks(struct gl_shader_program *prog,
       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;
       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;
+            }
+         }
+      }
+   }
+}
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 1d53b65..01b278c 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -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)
diff --git a/src/glsl/linker.h b/src/glsl/linker.h
index 7b1f6f9..b285877 100644
--- a/src/glsl/linker.h
+++ b/src/glsl/linker.h
@@ -69,6 +69,10 @@ validate_interstage_interface_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,
                                      struct gl_shader_program *prog);
-- 
1.8.4.2



More information about the mesa-dev mailing list