From 5699220cd5719be6fbafdefd75025a817bcb200a Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Wed, 2 Oct 2013 15:57:03 -0700 Subject: [PATCH] glsl: Exit when the shader IR contains an interface block instance While writing the link_varyings::single_interface_input test, I discovered that populate_consumer_input_sets assumes that all shader interface blocks have been lowered to discrete variables. Since there is a pass that does this, it is a reasonable assumption. It was, however, non-obvious. Make the code fail when it encounters such a thing, and add a test to verify that behavior. Signed-off-by: Ian Romanick --- src/glsl/link_varyings.cpp | 23 +++++++++---- src/glsl/tests/varyings_test.cpp | 71 ++++++++++++++++++++++++++-------------- 2 files changed, 63 insertions(+), 31 deletions(-) diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp index 3a6dfc8..8935c89 100644 --- a/src/glsl/link_varyings.cpp +++ b/src/glsl/link_varyings.cpp @@ -1038,7 +1038,7 @@ private: namespace linker { -void +bool populate_consumer_input_sets(void *mem_ctx, exec_list *ir, hash_table *consumer_inputs, hash_table *consumer_interface_inputs) @@ -1047,6 +1047,9 @@ populate_consumer_input_sets(void *mem_ctx, exec_list *ir, ir_variable *const input_var = ((ir_instruction *) node)->as_variable(); if ((input_var != NULL) && (input_var->data.mode == ir_var_shader_in)) { + if (input_var->type->is_interface()) + return false; + if (input_var->get_interface_type() != NULL) { char *const iface_field_name = ralloc_asprintf(mem_ctx, "%s.%s", @@ -1060,6 +1063,8 @@ populate_consumer_input_sets(void *mem_ctx, exec_list *ir, } } } + + return true; } } @@ -1116,11 +1121,17 @@ assign_varying_locations(struct gl_context *ctx, * not being inputs. This lets the optimizer eliminate them. */ - if (consumer) - linker::populate_consumer_input_sets(mem_ctx, - consumer->ir, - consumer_inputs, - consumer_interface_inputs); + if (consumer + && !linker::populate_consumer_input_sets(mem_ctx, + consumer->ir, + consumer_inputs, + consumer_interface_inputs)) { + assert(!"populate_consumer_input_sets failed"); + hash_table_dtor(tfeedback_candidates); + hash_table_dtor(consumer_inputs); + hash_table_dtor(consumer_interface_inputs); + return false; + } foreach_list(node, producer->ir) { ir_variable *const output_var = ((ir_instruction *) node)->as_variable(); diff --git a/src/glsl/tests/varyings_test.cpp b/src/glsl/tests/varyings_test.cpp index ab47fcf..76a08e3 100644 --- a/src/glsl/tests/varyings_test.cpp +++ b/src/glsl/tests/varyings_test.cpp @@ -35,7 +35,7 @@ */ namespace linker { -void +bool populate_consumer_input_sets(void *mem_ctx, exec_list *ir, hash_table *consumer_inputs, hash_table *consumer_interface_inputs); @@ -52,8 +52,8 @@ public: { return ralloc_asprintf(mem_ctx, "%s.%s", - simple_interface->name, - simple_interface->fields.structure[field].name); + iface->name, + iface->fields.structure[field].name); } void *mem_ctx; @@ -70,7 +70,11 @@ link_varyings::link_varyings() { glsl_type::vec(4), "v", - false + false, + 0, + 0, + 0, + 0 } }; @@ -151,10 +155,10 @@ TEST_F(link_varyings, single_simple_input) ir.push_tail(v); - linker::populate_consumer_input_sets(mem_ctx, - &ir, - consumer_inputs, - consumer_interface_inputs); + ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx, + &ir, + consumer_inputs, + consumer_interface_inputs)); EXPECT_EQ((void *) v, hash_table_find(consumer_inputs, "a")); EXPECT_EQ(1u, num_elements(consumer_inputs)); @@ -171,16 +175,16 @@ TEST_F(link_varyings, gl_ClipDistance) "gl_ClipDistance", ir_var_shader_in); - clipdistance->explicit_location = true; - clipdistance->location = VARYING_SLOT_CLIP_DIST0; - clipdistance->explicit_index = 0; + clipdistance->data.explicit_location = true; + clipdistance->data.location = VARYING_SLOT_CLIP_DIST0; + clipdistance->data.explicit_index = 0; ir.push_tail(clipdistance); - linker::populate_consumer_input_sets(mem_ctx, - &ir, - consumer_inputs, - consumer_interface_inputs); + ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx, + &ir, + consumer_inputs, + consumer_interface_inputs)); EXPECT_EQ((void *) clipdistance, hash_table_find(consumer_inputs, "gl_ClipDistance")); @@ -195,14 +199,14 @@ TEST_F(link_varyings, single_interface_input) simple_interface->fields.structure[0].name, ir_var_shader_in); - v->interface_type = simple_interface; + v->init_interface_type(simple_interface); ir.push_tail(v); - linker::populate_consumer_input_sets(mem_ctx, - &ir, - consumer_inputs, - consumer_interface_inputs); + ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx, + &ir, + consumer_inputs, + consumer_interface_inputs)); char *const full_name = interface_field_name(simple_interface); @@ -226,14 +230,14 @@ TEST_F(link_varyings, one_interface_and_one_simple_input) simple_interface->fields.structure[0].name, ir_var_shader_in); - iface->interface_type = simple_interface; + iface->init_interface_type(simple_interface); ir.push_tail(iface); - linker::populate_consumer_input_sets(mem_ctx, - &ir, - consumer_inputs, - consumer_interface_inputs); + ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx, + &ir, + consumer_inputs, + consumer_interface_inputs)); char *const iface_field_name = interface_field_name(simple_interface); @@ -244,3 +248,20 @@ TEST_F(link_varyings, one_interface_and_one_simple_input) EXPECT_EQ((void *) v, hash_table_find(consumer_inputs, "a")); EXPECT_EQ(1u, num_elements(consumer_inputs)); } + +TEST_F(link_varyings, invalid_interface_input) +{ + ir_variable *const v = + new(mem_ctx) ir_variable(simple_interface, + "named_interface", + ir_var_shader_in); + + ASSERT_EQ(simple_interface, v->get_interface_type()); + + ir.push_tail(v); + + EXPECT_FALSE(linker::populate_consumer_input_sets(mem_ctx, + &ir, + consumer_inputs, + consumer_interface_inputs)); +} -- 2.7.4