[Mesa-dev] [PATCH 13/19] linker: Modify cross_validate_outputs_to_inputs to match using explicit locations
Chia-I Wu
olvaffe at gmail.com
Thu Mar 27 23:36:03 PDT 2014
On Fri, Mar 28, 2014 at 5:40 AM, Ian Romanick <idr at freedesktop.org> wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> This will be used for GL_ARB_separate_shader_objects. That extension
> not only allows separable shaders to rendezvous by location, but it also
> allows traditionally linked shaders to rendezvous by location. The spec
> says:
>
> 36. How does the behavior of input/output interface matching differ
> between separable programs and non-separable programs?
>
> RESOLVED: The rules for matching individual variables or block
> members between stages are identical for separable and
> non-separable programs, with one exception -- matching variables
> of different type with the same location, as discussed in issue
> 34, applies only to separable programs.
>
> However, the ability to enforce matching requirements differs
> between program types. In non-separable programs, both sides of
> an interface are contained in the same linked program. In this
> case, if the linker detects a mismatch, it will generate a link
> error.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
> src/glsl/link_varyings.cpp | 73 +++++++++++++++++++++++++++++++++++-----
> src/glsl/tests/varyings_test.cpp | 35 ++++++++++++-------
> 2 files changed, 88 insertions(+), 20 deletions(-)
>
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index 3d9516c..3f80dbd 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -172,6 +172,7 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog,
> gl_shader *producer, gl_shader *consumer)
> {
> glsl_symbol_table parameters;
> + ir_variable *explicit_locations[MAX_VARYING] = { NULL, };
>
> /* Find all shader outputs in the "producer" stage.
> */
> @@ -181,7 +182,26 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog,
> if ((var == NULL) || (var->data.mode != ir_var_shader_out))
> continue;
>
> - parameters.add_variable(var);
> + if (!var->data.explicit_location
> + || var->data.location < VARYING_SLOT_VAR0)
> + parameters.add_variable(var);
> + else {
> + /* User-defined varyings with explicit locations are handled
> + * differently because they do not need to have matching names.
> + */
> + const unsigned idx = var->data.location - VARYING_SLOT_VAR0;
> +
> + if (explicit_locations[idx] != NULL) {
> + linker_error(prog,
> + "%s shader has multiple outputs explicitly "
> + "assigned to location %d\n",
> + _mesa_shader_stage_to_string(producer->Stage),
> + idx);
> + return;
> + }
> +
> + explicit_locations[idx] = var;
> + }
> }
>
>
> @@ -220,7 +240,27 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog,
> front_color, back_color,
> consumer->Stage, producer->Stage);
> } else {
> - ir_variable *const output = parameters.get_variable(input->name);
> + /* The rules for connecting inputs and outputs change in the presence
> + * of explicit locations. In this case, we no longer care about the
> + * names of the variables. Instead, we care only about the
> + * explicitly assigned location.
> + */
> + ir_variable *output = NULL;
> + if (input->data.explicit_location
> + && input->data.location >= VARYING_SLOT_VAR0) {
> + output = explicit_locations[input->data.location - VARYING_SLOT_VAR0];
> +
> + if (output == NULL) {
> + linker_error(prog,
> + "%s shader input `%s' with explicit location "
> + "has no matching output\n",
> + _mesa_shader_stage_to_string(consumer->Stage),
> + input->name);
> + }
> + } else {
> + output = parameters.get_variable(input->name);
> + }
> +
> if (output != NULL) {
> cross_validate_types_and_qualifiers(prog, input, output,
> consumer->Stage, producer->Stage);
> @@ -1052,8 +1092,13 @@ namespace linker {
> bool
> populate_consumer_input_sets(void *mem_ctx, exec_list *ir,
> hash_table *consumer_inputs,
> - hash_table *consumer_interface_inputs)
> + hash_table *consumer_interface_inputs,
> + ir_variable *consumer_inputs_with_locations[MAX_VARYING])
> {
> + memset(consumer_inputs_with_locations,
> + 0,
> + sizeof(consumer_inputs_with_locations[0]) * MAX_VARYING);
> +
> foreach_list(node, ir) {
> ir_variable *const input_var = ((ir_instruction *) node)->as_variable();
>
> @@ -1061,7 +1106,13 @@ populate_consumer_input_sets(void *mem_ctx, exec_list *ir,
> if (input_var->type->is_interface())
> return false;
>
> - if (input_var->get_interface_type() != NULL) {
> + if (input_var->data.user_location != -1) {
> + /* FINISHME: If the input is an interface, array, or structure, it
> + * FINISHME: will use multiple slots.
> + */
> + consumer_inputs_with_locations[input_var->data.user_location] =
> + input_var;
> + } else if (input_var->get_interface_type() != NULL) {
> char *const iface_field_name =
> ralloc_asprintf(mem_ctx, "%s.%s",
> input_var->get_interface_type()->name,
> @@ -1088,11 +1139,14 @@ ir_variable *
> get_matching_input(void *mem_ctx,
> const ir_variable *output_var,
> hash_table *consumer_inputs,
> - hash_table *consumer_interface_inputs)
> + hash_table *consumer_interface_inputs,
> + ir_variable *consumer_inputs_with_locations[MAX_VARYING])
> {
> ir_variable *input_var;
>
> - if (output_var->get_interface_type() != NULL) {
> + if (output_var->data.user_location != -1) {
> + input_var = consumer_inputs_with_locations[output_var->data.user_location];
> + } else if (output_var->get_interface_type() != NULL) {
> char *const iface_field_name =
> ralloc_asprintf(mem_ctx, "%s.%s",
> output_var->get_interface_type()->name,
> @@ -1213,6 +1267,7 @@ assign_varying_locations(struct gl_context *ctx,
> = hash_table_ctor(0, hash_table_string_hash, hash_table_string_compare);
> hash_table *consumer_interface_inputs
> = hash_table_ctor(0, hash_table_string_hash, hash_table_string_compare);
> + ir_variable *consumer_inputs_with_locations[MAX_VARYING];
>
> /* Operate in a total of four passes.
> *
> @@ -1239,7 +1294,8 @@ assign_varying_locations(struct gl_context *ctx,
> && !linker::populate_consumer_input_sets(mem_ctx,
> consumer->ir,
> consumer_inputs,
> - consumer_interface_inputs)) {
> + consumer_interface_inputs,
> + consumer_inputs_with_locations)) {
> assert(!"populate_consumer_input_sets failed");
> hash_table_dtor(tfeedback_candidates);
> hash_table_dtor(consumer_inputs);
> @@ -1261,7 +1317,8 @@ assign_varying_locations(struct gl_context *ctx,
>
> ir_variable *const input_var =
> linker::get_matching_input(mem_ctx, output_var, consumer_inputs,
> - consumer_interface_inputs);
> + consumer_interface_inputs,
> + consumer_inputs_with_locations);
consumer_inputs_with_locations is uninitialized when there is no
consumer. get_matching_input would return garbage when output_var has
a user location.
Something I ran into when testing the patches out.
>
> /* If a matching input variable was found, add this ouptut (and the
> * input) to the set. If this is a separable program and there is no
> diff --git a/src/glsl/tests/varyings_test.cpp b/src/glsl/tests/varyings_test.cpp
> index 1749112..8a188a7 100644
> --- a/src/glsl/tests/varyings_test.cpp
> +++ b/src/glsl/tests/varyings_test.cpp
> @@ -38,13 +38,15 @@ namespace linker {
> bool
> populate_consumer_input_sets(void *mem_ctx, exec_list *ir,
> hash_table *consumer_inputs,
> - hash_table *consumer_interface_inputs);
> + hash_table *consumer_interface_inputs,
> + ir_variable *consumer_inputs_with_locations[MAX_VARYING]);
>
> ir_variable *
> get_matching_input(void *mem_ctx,
> const ir_variable *output_var,
> hash_table *consumer_inputs,
> - hash_table *consumer_interface_inputs);
> + hash_table *consumer_interface_inputs,
> + ir_variable *consumer_inputs_with_locations[MAX_VARYING]);
> }
>
> class link_varyings : public ::testing::Test {
> @@ -68,6 +70,7 @@ public:
> hash_table *consumer_interface_inputs;
>
> const glsl_type *simple_interface;
> + ir_variable *junk[MAX_VARYING];
> };
>
> link_varyings::link_varyings()
> @@ -164,7 +167,8 @@ TEST_F(link_varyings, single_simple_input)
> ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx,
> &ir,
> consumer_inputs,
> - consumer_interface_inputs));
> + consumer_interface_inputs,
> + junk));
>
> EXPECT_EQ((void *) v, hash_table_find(consumer_inputs, "a"));
> EXPECT_EQ(1u, num_elements(consumer_inputs));
> @@ -190,7 +194,8 @@ TEST_F(link_varyings, gl_ClipDistance)
> ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx,
> &ir,
> consumer_inputs,
> - consumer_interface_inputs));
> + consumer_interface_inputs,
> + junk));
>
> EXPECT_EQ((void *) clipdistance,
> hash_table_find(consumer_inputs, "gl_ClipDistance"));
> @@ -212,8 +217,8 @@ TEST_F(link_varyings, single_interface_input)
> ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx,
> &ir,
> consumer_inputs,
> - consumer_interface_inputs));
> -
> + consumer_interface_inputs,
> + junk));
> char *const full_name = interface_field_name(simple_interface);
>
> EXPECT_EQ((void *) v, hash_table_find(consumer_interface_inputs, full_name));
> @@ -243,7 +248,8 @@ TEST_F(link_varyings, one_interface_and_one_simple_input)
> ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx,
> &ir,
> consumer_inputs,
> - consumer_interface_inputs));
> + consumer_interface_inputs,
> + junk));
>
> char *const iface_field_name = interface_field_name(simple_interface);
>
> @@ -269,7 +275,8 @@ TEST_F(link_varyings, invalid_interface_input)
> EXPECT_FALSE(linker::populate_consumer_input_sets(mem_ctx,
> &ir,
> consumer_inputs,
> - consumer_interface_inputs));
> + consumer_interface_inputs,
> + junk));
> }
>
> TEST_F(link_varyings, interface_field_doesnt_match_noninterface)
> @@ -288,7 +295,8 @@ TEST_F(link_varyings, interface_field_doesnt_match_noninterface)
> ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx,
> &ir,
> consumer_inputs,
> - consumer_interface_inputs));
> + consumer_interface_inputs,
> + junk));
>
> /* Create an output variable, "v", that is part of an interface block named
> * "a". They should not match.
> @@ -304,7 +312,8 @@ TEST_F(link_varyings, interface_field_doesnt_match_noninterface)
> linker::get_matching_input(mem_ctx,
> out_v,
> consumer_inputs,
> - consumer_interface_inputs);
> + consumer_interface_inputs,
> + junk);
>
> EXPECT_EQ(NULL, match);
> }
> @@ -328,7 +337,8 @@ TEST_F(link_varyings, interface_field_doesnt_match_noninterface_vice_versa)
> ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx,
> &ir,
> consumer_inputs,
> - consumer_interface_inputs));
> + consumer_interface_inputs,
> + junk));
>
> /* Create an output variable "a.v". They should not match.
> */
> @@ -341,7 +351,8 @@ TEST_F(link_varyings, interface_field_doesnt_match_noninterface_vice_versa)
> linker::get_matching_input(mem_ctx,
> out_v,
> consumer_inputs,
> - consumer_interface_inputs);
> + consumer_interface_inputs,
> + junk);
>
> EXPECT_EQ(NULL, match);
> }
> --
> 1.8.1.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
olv at LunarG.com
More information about the mesa-dev
mailing list