From abb51f449d938428779ac55d75a509f4158794cc Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Wed, 12 Jun 2024 14:01:50 +1000 Subject: [PATCH] nir: test opt_loop_merge_terminators() skips unhandled loops This test makes sure the merge if pass skips loops with trainling phis as those are not handled by the pass. Reviewed-by: Rhys Perry Part-of: --- src/compiler/nir/tests/opt_loop_tests.cpp | 98 +++++++++++++++++++++-- 1 file changed, 91 insertions(+), 7 deletions(-) diff --git a/src/compiler/nir/tests/opt_loop_tests.cpp b/src/compiler/nir/tests/opt_loop_tests.cpp index 3bfa2afd670..20b26a49a9b 100644 --- a/src/compiler/nir/tests/opt_loop_tests.cpp +++ b/src/compiler/nir/tests/opt_loop_tests.cpp @@ -27,6 +27,10 @@ class nir_opt_loop_test : public nir_test { protected: nir_opt_loop_test(); + nir_deref_instr *add_loop_terminators(nir_if **term1, nir_if **term2); + void create_loop_phis(nir_loop *loop, nir_if *term1, nir_if *term2, + nir_def *def1, nir_def *def2); + nir_def *in_def; nir_variable *out_var; nir_variable *ubo_var; @@ -43,13 +47,9 @@ nir_opt_loop_test::nir_opt_loop_test() out_var = nir_variable_create(b->shader, nir_var_shader_out, glsl_int_type(), "out"); } -TEST_F(nir_opt_loop_test, opt_loop_merge_terminators_deref_after_first_if) +nir_deref_instr * +nir_opt_loop_test::add_loop_terminators(nir_if **term1, nir_if **term2) { - /* Tests that opt_loop_merge_terminators creates valid nir after it merges - * terminators that have a deref statement between them: - */ - nir_loop *loop = nir_push_loop(b); - /* Add first terminator */ nir_def *one = nir_imm_int(b, 1); nir_def *cmp_result = nir_ieq(b, in_def, one); @@ -57,6 +57,9 @@ TEST_F(nir_opt_loop_test, opt_loop_merge_terminators_deref_after_first_if) nir_jump(b, nir_jump_break); nir_pop_if(b, nif); + if (term1) + *term1 = nif; + nir_deref_instr *deref = nir_build_deref_var(b, ubo_var); nir_def *ubo_def = nir_load_deref(b, deref); @@ -67,11 +70,40 @@ TEST_F(nir_opt_loop_test, opt_loop_merge_terminators_deref_after_first_if) nir_jump(b, nir_jump_break); nir_pop_if(b, nif2); + if (term2) + *term2 = nif2; + + return deref; +} + +void +nir_opt_loop_test::create_loop_phis(nir_loop *loop, + nir_if *term1, nir_if *term2, + nir_def *def1, nir_def *def2) +{ + nir_phi_instr *phi_instr = nir_phi_instr_create(b->shader); + nir_def_init(&phi_instr->instr, &phi_instr->def, 1, 32); + nir_phi_instr_add_src(phi_instr, nir_if_first_then_block(term1), def1); + nir_phi_instr_add_src(phi_instr, nir_if_first_then_block(term2), def2); + + nir_instr_insert(nir_after_cf_node(&loop->cf_node), + &phi_instr->instr); +} + +TEST_F(nir_opt_loop_test, opt_loop_merge_terminators_deref_after_first_if) +{ + /* Tests that opt_loop_merge_terminators creates valid nir after it merges + * terminators that have a deref statement between them: + */ + nir_loop *loop = nir_push_loop(b); + + nir_deref_instr *deref = add_loop_terminators(NULL, NULL); + /* Load from deref that will be moved inside the continue branch of the * first if-statements continue block. If not handled correctly during * the merge this will fail nir validation. */ - ubo_def = nir_load_deref(b, deref); + nir_def *ubo_def = nir_load_deref(b, deref); nir_store_var(b, out_var, ubo_def, 1); nir_pop_loop(b, loop); @@ -80,3 +112,55 @@ TEST_F(nir_opt_loop_test, opt_loop_merge_terminators_deref_after_first_if) nir_validate_shader(b->shader, NULL); } + +TEST_F(nir_opt_loop_test, opt_loop_merge_terminators_skip_merge_if_phis) +{ + /* Tests that opt_loop_merge_terminators skips merging the terminators if + * the loop has phis. We can update or remove this test if support for + * phis is added to this pass: + */ + nir_deref_instr *deref = nir_build_deref_var(b, ubo_var); + nir_def *ubo_def = nir_load_deref(b, deref); + + nir_loop *loop = nir_push_loop(b); + + nir_if *term1; + nir_if *term2; + add_loop_terminators(&term1, &term2); + + nir_pop_loop(b, loop); + + create_loop_phis(loop, term1, term2, in_def, ubo_def); + + ASSERT_FALSE(nir_opt_loop(b->shader)); + + nir_validate_shader(b->shader, NULL); +} + +TEST_F(nir_opt_loop_test, opt_loop_merge_terminators_skip_merge_if_phis_nested_loop) +{ + /* Tests that opt_loop_merge_terminators skips merging the terminators if + * the loop has phis. We can update or remove this test if support for + * phis is added to this pass: + */ + nir_deref_instr *deref = nir_build_deref_var(b, ubo_var); + nir_def *ubo_def = nir_load_deref(b, deref); + + nir_loop *loop = nir_push_loop(b); + + /* Add a nested loop to make sure we test the correct loop for trailing phis */ + nir_loop *nested_loop = nir_push_loop(b); + nir_pop_loop(b, nested_loop); + + nir_if *term1; + nir_if *term2; + add_loop_terminators(&term1, &term2); + + nir_pop_loop(b, loop); + + create_loop_phis(loop, term1, term2, in_def, ubo_def); + + ASSERT_FALSE(nir_opt_loop(b->shader)); + + nir_validate_shader(b->shader, NULL); +}