From c1bbaff1e83f901d67d78f9e1ddfe8291dd09bfa Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Wed, 3 Feb 2016 09:39:10 +1100 Subject: [PATCH] glsl: replace unreachable code with an assert() All interface blocks will have been lowered by this point so just use an assert. Returning false would have caused all sorts of problems if they were not lowered yet and there is an assert to catch this later anyway. We also update the tests to reflect this change. Reviewed-by: Kenneth Graunke --- src/compiler/glsl/link_varyings.cpp | 25 +++----- src/compiler/glsl/tests/varyings_test.cpp | 78 +++++++++-------------- 2 files changed, 38 insertions(+), 65 deletions(-) diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp index a4c730ffdcf..535c83cd0e7 100644 --- a/src/compiler/glsl/link_varyings.cpp +++ b/src/compiler/glsl/link_varyings.cpp @@ -1352,7 +1352,7 @@ private: namespace linker { -bool +void populate_consumer_input_sets(void *mem_ctx, exec_list *ir, hash_table *consumer_inputs, hash_table *consumer_interface_inputs, @@ -1366,8 +1366,8 @@ populate_consumer_input_sets(void *mem_ctx, exec_list *ir, ir_variable *const input_var = node->as_variable(); if ((input_var != NULL) && (input_var->data.mode == ir_var_shader_in)) { - if (input_var->type->is_interface()) - return false; + /* All interface blocks should have been lowered by this point */ + assert(!input_var->type->is_interface()); if (input_var->data.explicit_location) { /* assign_varying_locations only cares about finding the @@ -1401,8 +1401,6 @@ populate_consumer_input_sets(void *mem_ctx, exec_list *ir, } } } - - return true; } /** @@ -1626,18 +1624,11 @@ assign_varying_locations(struct gl_context *ctx, if (producer) canonicalize_shader_io(producer->ir, ir_var_shader_out); - if (consumer - && !linker::populate_consumer_input_sets(mem_ctx, - consumer->ir, - consumer_inputs, - consumer_interface_inputs, - consumer_inputs_with_locations)) { - assert(!"populate_consumer_input_sets failed"); - hash_table_dtor(tfeedback_candidates); - hash_table_dtor(consumer_inputs); - hash_table_dtor(consumer_interface_inputs); - return false; - } + if (consumer) + linker::populate_consumer_input_sets(mem_ctx, consumer->ir, + consumer_inputs, + consumer_interface_inputs, + consumer_inputs_with_locations); if (producer) { foreach_in_list(ir_instruction, node, producer->ir) { diff --git a/src/compiler/glsl/tests/varyings_test.cpp b/src/compiler/glsl/tests/varyings_test.cpp index 0c4e0a471b8..9be5e8344b4 100644 --- a/src/compiler/glsl/tests/varyings_test.cpp +++ b/src/compiler/glsl/tests/varyings_test.cpp @@ -156,11 +156,11 @@ TEST_F(link_varyings, single_simple_input) ir.push_tail(v); - ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx, - &ir, - consumer_inputs, - consumer_interface_inputs, - junk)); + linker::populate_consumer_input_sets(mem_ctx, + &ir, + consumer_inputs, + consumer_interface_inputs, + junk); EXPECT_EQ((void *) v, hash_table_find(consumer_inputs, "a")); EXPECT_EQ(1u, num_elements(consumer_inputs)); @@ -183,11 +183,11 @@ TEST_F(link_varyings, gl_ClipDistance) ir.push_tail(clipdistance); - ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx, - &ir, - consumer_inputs, - consumer_interface_inputs, - junk)); + linker::populate_consumer_input_sets(mem_ctx, + &ir, + consumer_inputs, + consumer_interface_inputs, + junk); EXPECT_EQ(clipdistance, junk[VARYING_SLOT_CLIP_DIST0]); EXPECT_TRUE(is_empty(consumer_inputs)); @@ -205,11 +205,11 @@ TEST_F(link_varyings, single_interface_input) ir.push_tail(v); - ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx, - &ir, - consumer_inputs, - consumer_interface_inputs, - junk)); + linker::populate_consumer_input_sets(mem_ctx, + &ir, + consumer_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)); @@ -236,11 +236,11 @@ TEST_F(link_varyings, one_interface_and_one_simple_input) ir.push_tail(iface); - ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx, - &ir, - consumer_inputs, - consumer_interface_inputs, - junk)); + linker::populate_consumer_input_sets(mem_ctx, + &ir, + consumer_inputs, + consumer_interface_inputs, + junk); char *const iface_field_name = interface_field_name(simple_interface); @@ -252,24 +252,6 @@ TEST_F(link_varyings, one_interface_and_one_simple_input) 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, - junk)); -} - TEST_F(link_varyings, interface_field_doesnt_match_noninterface) { char *const iface_field_name = interface_field_name(simple_interface); @@ -283,11 +265,11 @@ TEST_F(link_varyings, interface_field_doesnt_match_noninterface) ir.push_tail(in_v); - ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx, - &ir, - consumer_inputs, - consumer_interface_inputs, - junk)); + linker::populate_consumer_input_sets(mem_ctx, + &ir, + consumer_inputs, + consumer_interface_inputs, + junk); /* Create an output variable, "v", that is part of an interface block named * "a". They should not match. @@ -325,11 +307,11 @@ TEST_F(link_varyings, interface_field_doesnt_match_noninterface_vice_versa) ir.push_tail(in_v); - ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx, - &ir, - consumer_inputs, - consumer_interface_inputs, - junk)); + linker::populate_consumer_input_sets(mem_ctx, + &ir, + consumer_inputs, + consumer_interface_inputs, + junk); /* Create an output variable "a.v". They should not match. */ -- 2.30.2