From acce4ce04ec1b591b47afeda512a9b554dac14f3 Mon Sep 17 00:00:00 2001 From: Caio Marcelo de Oliveira Filho Date: Tue, 15 Dec 2020 23:25:51 -0800 Subject: [PATCH] nir: Consider pointer initializers in nir_remove_dead_variables Between the creation of a shader (from GLSL or SPIRV frontends) and nir_lower_variable_initializers is called, variables may refer to other variables for initialization. Those referred variables need to be kept alive, so consider that in the pass. Fixes: 7acc81056f7 ("compiler/nir: Add support for variable initialization from a pointer") Reviewed-by: Jason Ekstrand Part-of: --- src/compiler/nir/nir_remove_dead_variables.c | 21 ++++++--- src/compiler/nir/tests/vars_tests.cpp | 48 ++++++++++++++++++++ 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/compiler/nir/nir_remove_dead_variables.c b/src/compiler/nir/nir_remove_dead_variables.c index deaadf41d66..fde006f917f 100644 --- a/src/compiler/nir/nir_remove_dead_variables.c +++ b/src/compiler/nir/nir_remove_dead_variables.c @@ -67,14 +67,21 @@ add_var_use_deref(nir_deref_instr *deref, struct set *live) if (deref->deref_type != nir_deref_type_var) return; - /* If it's not a local that never escapes the shader, then any access at - * all means we need to keep it alive. + /* Since these local variables don't escape the shader, writing doesn't + * make them live. Only keep them if they are used by some intrinsic. */ - if (!(deref->var->data.mode & (nir_var_function_temp | - nir_var_shader_temp | - nir_var_mem_shared)) || - deref_used_for_not_store(deref)) - _mesa_set_add(live, deref->var); + if ((deref->var->data.mode & (nir_var_function_temp | + nir_var_shader_temp | + nir_var_mem_shared)) && + !deref_used_for_not_store(deref)) + return; + + nir_variable *var = deref->var; + do { + _mesa_set_add(live, var); + /* Also mark the chain of variables used to initialize it. */ + var = var->pointer_initializer; + } while (var); } static void diff --git a/src/compiler/nir/tests/vars_tests.cpp b/src/compiler/nir/tests/vars_tests.cpp index 91bd6974cb6..eb5d106839a 100644 --- a/src/compiler/nir/tests/vars_tests.cpp +++ b/src/compiler/nir/tests/vars_tests.cpp @@ -196,6 +196,7 @@ class nir_copy_prop_vars_test : public nir_vars_test {}; class nir_dead_write_vars_test : public nir_vars_test {}; class nir_combine_stores_test : public nir_vars_test {}; class nir_split_vars_test : public nir_vars_test {}; +class nir_remove_dead_variables_test : public nir_vars_test {}; } // namespace @@ -2184,3 +2185,50 @@ TEST_F(nir_split_vars_test, split_wildcard_copy) ASSERT_EQ(count_function_temp_vars(), 8); ASSERT_EQ(count_intrinsics(nir_intrinsic_copy_deref), 4); } + +TEST_F(nir_remove_dead_variables_test, pointer_initializer_used) +{ + nir_variable *x = create_int(nir_var_shader_temp, "x"); + nir_variable *y = create_int(nir_var_shader_temp, "y"); + y->pointer_initializer = x; + nir_variable *out = create_int(nir_var_shader_out, "out"); + + nir_validate_shader(b->shader, NULL); + + nir_copy_var(b, out, y); + + bool progress = nir_remove_dead_variables(b->shader, nir_var_all, NULL); + EXPECT_FALSE(progress); + + nir_validate_shader(b->shader, NULL); + + unsigned count = 0; + nir_foreach_variable_in_shader(var, b->shader) + count++; + + ASSERT_EQ(count, 3); +} + +TEST_F(nir_remove_dead_variables_test, pointer_initializer_dead) +{ + nir_variable *x = create_int(nir_var_shader_temp, "x"); + nir_variable *y = create_int(nir_var_shader_temp, "y"); + nir_variable *z = create_int(nir_var_shader_temp, "z"); + y->pointer_initializer = x; + z->pointer_initializer = y; + + nir_validate_shader(b->shader, NULL); + + bool progress = nir_remove_dead_variables(b->shader, nir_var_all, NULL); + EXPECT_TRUE(progress); + + nir_validate_shader(b->shader, NULL); + + unsigned count = 0; + nir_foreach_variable_in_shader(var, b->shader) + count++; + + ASSERT_EQ(count, 0); +} + +