[Mesa-dev] [PATCH 6/6] glsl: Simplify the interface to link_invalidate_variable_locations
Ian Romanick
idr at freedesktop.org
Fri Oct 11 20:18:41 CEST 2013
From: Ian Romanick <ian.d.romanick at intel.com>
The unit tests added in the previous commits prove some things about the
state of some internal data structures. The most important of these is
that all built-in input and output variables have explicit_location
set. This means that link_invalidate_variable_locations doesn't need to
know the range of non-generic shader inputs or outputs. It can simply
reset location state depending on whether explicit_location is set.
There are two additional assumptions that were already implicit in the
code that comments now document.
- ir_variable::is_unmatched_generic_inout is only used by the linker
when connecting outputs from one shader stage to inputs of another
shader stage.
- Any varying that has explicit_location set must be a built-in. This
will be true until GL_ARB_separate_shader_objects is supported.
As a result, the input_base and output_base parameters to
link_invalidate_variable_locations are no longer necessary, and the code
for resetting locations and setting is_unmatched_generic_inout can be
simplified.
Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
Cc: Paul Berry <stereotype441 at gmail.com>
---
src/glsl/linker.cpp | 48 ++++++++++-----------
src/glsl/linker.h | 3 +-
src/glsl/tests/invalidate_locations_test.cpp | 62 ++++++++++++++--------------
3 files changed, 55 insertions(+), 58 deletions(-)
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 02eb4e1..acc3039 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -366,8 +366,7 @@ parse_program_resource_name(const GLchar *name,
void
-link_invalidate_variable_locations(exec_list *ir, int input_base,
- int output_base)
+link_invalidate_variable_locations(exec_list *ir)
{
foreach_list(node, ir) {
ir_variable *const var = ((ir_instruction *) node)->as_variable();
@@ -375,26 +374,30 @@ link_invalidate_variable_locations(exec_list *ir, int input_base,
if (var == NULL)
continue;
- int base;
- switch (var->mode) {
- case ir_var_shader_in:
- base = input_base;
- break;
- case ir_var_shader_out:
- base = output_base;
- break;
- default:
- continue;
- }
-
- /* Only assign locations for generic attributes / varyings / etc.
+ /* Only assign locations for variables that lack an explicit location.
+ * Explicit locations are set for all built-in variables, generic vertex
+ * shader inputs (via layout(location=...)), and generic fragment shader
+ * outputs (also via layout(location=...)).
*/
- if ((var->location >= base) && !var->explicit_location)
+ if (!var->explicit_location) {
var->location = -1;
+ var->location_frac = 0;
+ }
- if ((var->location == -1) && !var->explicit_location) {
+ /* ir_variable::is_unmatched_generic_inout is used by the linker while
+ * connecting outputs from one stage to inputs of the next stage.
+ *
+ * There are two implicit assumptions here. First, we assume that any
+ * built-in variable (i.e., non-generic in or out) will have
+ * explicit_location set. Second, we assume that any generic in or out
+ * will not have explicit_location set.
+ *
+ * This second assumption will only be valid until
+ * GL_ARB_separate_shader_objects is supported. When that extension is
+ * implemented, this function will need some modifications.
+ */
+ if (!var->explicit_location) {
var->is_unmatched_generic_inout = 1;
- var->location_frac = 0;
} else {
var->is_unmatched_generic_inout = 0;
}
@@ -2221,18 +2224,15 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
/* Mark all generic shader inputs and outputs as unpaired. */
if (prog->_LinkedShaders[MESA_SHADER_VERTEX] != NULL) {
link_invalidate_variable_locations(
- prog->_LinkedShaders[MESA_SHADER_VERTEX]->ir,
- VERT_ATTRIB_GENERIC0, VARYING_SLOT_VAR0);
+ prog->_LinkedShaders[MESA_SHADER_VERTEX]->ir);
}
if (prog->_LinkedShaders[MESA_SHADER_GEOMETRY] != NULL) {
link_invalidate_variable_locations(
- prog->_LinkedShaders[MESA_SHADER_GEOMETRY]->ir,
- VARYING_SLOT_VAR0, VARYING_SLOT_VAR0);
+ prog->_LinkedShaders[MESA_SHADER_GEOMETRY]->ir);
}
if (prog->_LinkedShaders[MESA_SHADER_FRAGMENT] != NULL) {
link_invalidate_variable_locations(
- prog->_LinkedShaders[MESA_SHADER_FRAGMENT]->ir,
- VARYING_SLOT_VAR0, FRAG_RESULT_DATA0);
+ prog->_LinkedShaders[MESA_SHADER_FRAGMENT]->ir);
}
/* FINISHME: The value of the max_attribute_index parameter is
diff --git a/src/glsl/linker.h b/src/glsl/linker.h
index 9915c38..887cd33 100644
--- a/src/glsl/linker.h
+++ b/src/glsl/linker.h
@@ -31,8 +31,7 @@ link_function_calls(gl_shader_program *prog, gl_shader *main,
gl_shader **shader_list, unsigned num_shaders);
extern void
-link_invalidate_variable_locations(exec_list *ir, int input_base,
- int output_base);
+link_invalidate_variable_locations(exec_list *ir);
extern void
link_assign_uniform_locations(struct gl_shader_program *prog);
diff --git a/src/glsl/tests/invalidate_locations_test.cpp b/src/glsl/tests/invalidate_locations_test.cpp
index 958acec..b26d825 100644
--- a/src/glsl/tests/invalidate_locations_test.cpp
+++ b/src/glsl/tests/invalidate_locations_test.cpp
@@ -72,9 +72,7 @@ TEST_F(invalidate_locations, simple_vertex_in_generic)
ir.push_tail(var);
- link_invalidate_variable_locations(&ir,
- VERT_ATTRIB_GENERIC0,
- VARYING_SLOT_VAR0);
+ link_invalidate_variable_locations(&ir);
EXPECT_EQ(-1, var->location);
EXPECT_EQ(0u, var->location_frac);
@@ -97,9 +95,7 @@ TEST_F(invalidate_locations, explicit_location_vertex_in_generic)
ir.push_tail(var);
- link_invalidate_variable_locations(&ir,
- VERT_ATTRIB_GENERIC0,
- VARYING_SLOT_VAR0);
+ link_invalidate_variable_locations(&ir);
EXPECT_EQ(VERT_ATTRIB_GENERIC0, var->location);
EXPECT_EQ(0u, var->location_frac);
@@ -123,9 +119,7 @@ TEST_F(invalidate_locations, explicit_location_frac_vertex_in_generic)
ir.push_tail(var);
- link_invalidate_variable_locations(&ir,
- VERT_ATTRIB_GENERIC0,
- VARYING_SLOT_VAR0);
+ link_invalidate_variable_locations(&ir);
EXPECT_EQ(VERT_ATTRIB_GENERIC0, var->location);
EXPECT_EQ(2u, var->location_frac);
@@ -148,9 +142,7 @@ TEST_F(invalidate_locations, vertex_in_builtin)
ir.push_tail(var);
- link_invalidate_variable_locations(&ir,
- VERT_ATTRIB_GENERIC0,
- VARYING_SLOT_VAR0);
+ link_invalidate_variable_locations(&ir);
EXPECT_EQ(VERT_ATTRIB_POS, var->location);
EXPECT_EQ(0u, var->location_frac);
@@ -162,8 +154,15 @@ TEST_F(invalidate_locations, vertex_in_builtin_without_explicit)
{
/* This test is almost identical to vertex_in_builtin. However,
* ir_variable::explicit_location is not.
- * link_invalidate_variable_locations has the behavior that non-generic
- * inputs (or outputs) are not modified.
+ *
+ * At one point in time, link_invalidate_variable_locations would not
+ * modify any non-generic inputs (or outputs). However, the
+ * vertex_builtin::inputs_have_explicit_location et al. test cases prove
+ * that all built-in variables have explicit_location set.
+ * link_invalidate_variable_locations will reset the locations of any
+ * variable that does not have explicit_location set.
+ *
+ * In addition, is_unmatch_generic_inout depend only on explicit_location.
*/
ir_variable *const var =
new(mem_ctx) ir_variable(glsl_type::vec(4),
@@ -177,14 +176,12 @@ TEST_F(invalidate_locations, vertex_in_builtin_without_explicit)
ir.push_tail(var);
- link_invalidate_variable_locations(&ir,
- VERT_ATTRIB_GENERIC0,
- VARYING_SLOT_VAR0);
+ link_invalidate_variable_locations(&ir);
- EXPECT_EQ(VERT_ATTRIB_POS, var->location);
+ EXPECT_EQ(-1, var->location);
EXPECT_EQ(0u, var->location_frac);
EXPECT_FALSE(var->explicit_location);
- EXPECT_FALSE(var->is_unmatched_generic_inout);
+ EXPECT_TRUE(var->is_unmatched_generic_inout);
}
TEST_F(invalidate_locations, simple_vertex_out_generic)
@@ -201,9 +198,7 @@ TEST_F(invalidate_locations, simple_vertex_out_generic)
ir.push_tail(var);
- link_invalidate_variable_locations(&ir,
- VERT_ATTRIB_GENERIC0,
- VARYING_SLOT_VAR0);
+ link_invalidate_variable_locations(&ir);
EXPECT_EQ(-1, var->location);
EXPECT_EQ(0u, var->location_frac);
@@ -226,9 +221,7 @@ TEST_F(invalidate_locations, vertex_out_builtin)
ir.push_tail(var);
- link_invalidate_variable_locations(&ir,
- VERT_ATTRIB_GENERIC0,
- VARYING_SLOT_VAR0);
+ link_invalidate_variable_locations(&ir);
EXPECT_EQ(VARYING_SLOT_COL0, var->location);
EXPECT_EQ(0u, var->location_frac);
@@ -240,8 +233,15 @@ TEST_F(invalidate_locations, vertex_out_builtin_without_explicit)
{
/* This test is almost identical to vertex_out_builtin. However,
* ir_variable::explicit_location is not.
- * link_invalidate_variable_locations has the behavior that non-generic
- * inputs (or outputs) are not modified.
+ *
+ * At one point in time, link_invalidate_variable_locations would not
+ * modify any non-generic inputs (or outputs). However, the
+ * vertex_builtin::inputs_have_explicit_location et al. test cases prove
+ * that all built-in variables have explicit_location set.
+ * link_invalidate_variable_locations will reset the locations of any
+ * variable that does not have explicit_location set.
+ *
+ * In addition, is_unmatch_generic_inout depend only on explicit_location.
*/
ir_variable *const var =
new(mem_ctx) ir_variable(glsl_type::vec(4),
@@ -255,12 +255,10 @@ TEST_F(invalidate_locations, vertex_out_builtin_without_explicit)
ir.push_tail(var);
- link_invalidate_variable_locations(&ir,
- VERT_ATTRIB_GENERIC0,
- VARYING_SLOT_VAR0);
+ link_invalidate_variable_locations(&ir);
- EXPECT_EQ(VARYING_SLOT_COL0, var->location);
+ EXPECT_EQ(-1, var->location);
EXPECT_EQ(0u, var->location_frac);
EXPECT_FALSE(var->explicit_location);
- EXPECT_FALSE(var->is_unmatched_generic_inout);
+ EXPECT_TRUE(var->is_unmatched_generic_inout);
}
--
1.8.1.4
More information about the mesa-dev
mailing list