On 05/16/2013 05:44 PM, Jordan Justen wrote:
Verify that interface blocks match when linking separate shader
stages into a program.
Fixes piglit glsl-1.50 tests:
* linker/interface-blocks-vs-fs-member-count-mismatch.shader_test
* linker/interface-blocks-vs-fs-member-order-mismatch.shader_test
Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com>
---
src/glsl/interface_blocks.cpp | 47 +++++++++++++++++++++++++++++++++++++++--
src/glsl/interface_blocks.h | 4 ++++
src/glsl/linker.cpp | 6 ++++++
3 files changed, 55 insertions(+), 2 deletions(-)
In my mind, validating interface blocks within a single stage and
between two adjacent stages are really separate tasks that work
differently. Intrastage has an arbitrarily long list of, say, vertex
shaders; interstage has exactly two.
I really dislike how this patch conflates the two. I also find creating
a list with exactly two elements, looping over it, and special-casing
every element to be quite ugly.
Sorry for taking so long on this series and coming in with harsh
feedback. :(
I'll send out a proposed v5 that does this a bit differently and still
passes all of your piglit tests.
diff --git a/src/glsl/interface_blocks.cpp b/src/glsl/interface_blocks.cpp
index 6dfd0c4..b458b59 100644
--- a/src/glsl/interface_blocks.cpp
+++ b/src/glsl/interface_blocks.cpp
@@ -30,14 +30,21 @@
#include "glsl_symbol_table.h"
#include "main/macros.h"
#include "program/hash_table.h"
+#include "linker.h"
static bool
cross_validate_interface_blocks(const gl_shader **shader_list,
- unsigned num_shaders)
+ unsigned num_shaders,
+ bool interstage)
{
bool ok = true;
glsl_symbol_table interfaces;
+ /* Interstage linking checks assume 2 shaders. First the producer, and
+ * then the consumer.
+ */
+ assert(!interstage || num_shaders == 2);
+
for (unsigned int i = 0; ok && i < num_shaders; i++) {
if (shader_list[i] == NULL)
continue;
@@ -52,6 +59,18 @@ cross_validate_interface_blocks(const gl_shader
**shader_list,
if (iface_type == NULL)
continue;
+ /* If we are checking interstage linking then we assume:
+ * * The first shader (producer) has i == 0, and for the
+ * producer we don't need to check input interfaces.
+ * * The second shader (consumer) has i == 1, and for the
+ * consumer we don't need to check output interfaces.
+ */
+ if (interstage &&
+ ((var->mode == ir_var_shader_in && i == 0) ||
+ (var->mode == ir_var_shader_out && i == 1))
+ )
+ continue;
+
const char *iface_name = iface_type->name;
const glsl_type *old_iface_type =
@@ -70,6 +89,18 @@ cross_validate_interface_blocks(const gl_shader
**shader_list,
ok &= old_iface_type == iface_type;
if (!ok)
break;
+
+ /* For interstage linking, if the interface is an input, then
+ * we need to verify that its type matches a previously declared
+ * output type.
+ */
+ if (interstage && var->mode == ir_var_shader_in) {
+ old_iface_type =
+ interfaces.get_interface(iface_name, ir_var_shader_out);
+ ok &= old_iface_type == iface_type;
+ if (!ok)
+ break;
+ }
}
}
@@ -81,6 +112,18 @@ validate_intrastage_interface_blocks(const gl_shader
**shader_list,
unsigned num_shaders)
{
return cross_validate_interface_blocks(shader_list,
- num_shaders);
+ num_shaders,
+ false);
+}
+
+bool
+validate_interstage_interface_blocks(const gl_shader *producer,
+ const gl_shader *consumer)
+{
+ const gl_shader *shader_list[] = { producer, consumer };
+
+ return cross_validate_interface_blocks((const gl_shader **) shader_list,
+ ARRAY_SIZE(shader_list),
+ true);
}
diff --git a/src/glsl/interface_blocks.h b/src/glsl/interface_blocks.h
index 4c37c02..58c847c 100644
--- a/src/glsl/interface_blocks.h
+++ b/src/glsl/interface_blocks.h
@@ -29,4 +29,8 @@ bool
validate_intrastage_interface_blocks(const gl_shader **shader_list,
unsigned num_shaders);
+bool
+validate_interstage_interface_blocks(const gl_shader *producer,
+ const gl_shader *consumer);
+
#endif /* GLSL_INTERFACE_BLOCKS_H */
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 4f2cab2..71203b1 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1729,6 +1729,12 @@ link_shaders(struct gl_context *ctx, struct
gl_shader_program *prog)
if (prog->_LinkedShaders[i] == NULL)
continue;
+ if (!validate_interstage_interface_blocks(prog->_LinkedShaders[prev],
+ prog->_LinkedShaders[i])) {
+ linker_error(prog, "interface block mismatch between shader
stages\n");
+ goto done;
+ }
+
if (!cross_validate_outputs_to_inputs(prog,
prog->_LinkedShaders[prev],
prog->_LinkedShaders[i]))
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
