diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build index d9899d29fc2..dad896dcf75 100644 --- a/src/compiler/nir/meson.build +++ b/src/compiler/nir/meson.build @@ -458,4 +458,17 @@ if with_tests ), suite : ['compiler', 'nir'], ) + + test( + 'nir_lower_returns', + executable( + 'nir_lower_returns_tests', + files('tests/lower_returns_tests.cpp'), + cpp_args : [cpp_msvc_compat_args], + gnu_symbol_visibility : 'hidden', + include_directories : [inc_include, inc_src, inc_mapi, inc_mesa, inc_gallium, inc_gallium_aux], + dependencies : [dep_thread, idep_gtest, idep_nir, idep_mesautil], + ), + suite : ['compiler', 'nir'], + ) endif diff --git a/src/compiler/nir/nir_control_flow.c b/src/compiler/nir/nir_control_flow.c index 75146cc1cf3..9d5f52000d7 100644 --- a/src/compiler/nir/nir_control_flow.c +++ b/src/compiler/nir/nir_control_flow.c @@ -226,8 +226,8 @@ rewrite_phi_preds(nir_block *block, nir_block *old_pred, nir_block *new_pred) } } -static void -insert_phi_undef(nir_block *block, nir_block *pred) +void +nir_insert_phi_undef(nir_block *block, nir_block *pred) { nir_function_impl *impl = nir_cf_node_get_function(&block->cf_node); nir_foreach_instr(instr, block) { @@ -298,7 +298,7 @@ block_add_normal_succs(nir_block *block) nir_block *head_block = nir_loop_first_block(loop); link_blocks(block, head_block, NULL); - insert_phi_undef(head_block, block); + nir_insert_phi_undef(head_block, block); } else { nir_function_impl *impl = nir_cf_node_as_function(parent); link_blocks(block, impl->end_block, NULL); @@ -318,7 +318,7 @@ block_add_normal_succs(nir_block *block) nir_block *first_block = nir_loop_first_block(next_loop); link_blocks(block, first_block, NULL); - insert_phi_undef(first_block, block); + nir_insert_phi_undef(first_block, block); } } } diff --git a/src/compiler/nir/nir_control_flow.h b/src/compiler/nir/nir_control_flow.h index 9111b30a297..c1867a81180 100644 --- a/src/compiler/nir/nir_control_flow.h +++ b/src/compiler/nir/nir_control_flow.h @@ -171,6 +171,9 @@ nir_cf_node_remove(nir_cf_node *node) nir_cf_delete(&list); } +/** inserts undef phi sources from predcessor into phis of the block */ +void nir_insert_phi_undef(nir_block *block, nir_block *pred); + #ifdef __cplusplus } #endif diff --git a/src/compiler/nir/nir_lower_returns.c b/src/compiler/nir/nir_lower_returns.c index ae933e662cd..76b320b8042 100644 --- a/src/compiler/nir/nir_lower_returns.c +++ b/src/compiler/nir/nir_lower_returns.c @@ -62,6 +62,9 @@ predicate_following(nir_cf_node *node, struct lower_returns_state *state) * conditional break. */ nir_jump(b, nir_jump_break); + + nir_block *block = nir_cursor_current_block(b->cursor); + nir_insert_phi_undef(block->successors[0], block); } else { /* Otherwise, we need to actually move everything into the else case * of the if statement. @@ -205,6 +208,8 @@ lower_returns_in_block(nir_block *block, struct lower_returns_state *state) if (state->loop) { /* We're in a loop; we need to break out of it. */ nir_jump(b, nir_jump_break); + + nir_insert_phi_undef(block->successors[0], block); } else { /* Not in a loop; we'll deal with predicating later*/ assert(nir_cf_node_next(&block->cf_node) == NULL); diff --git a/src/compiler/nir/tests/lower_returns_tests.cpp b/src/compiler/nir/tests/lower_returns_tests.cpp new file mode 100644 index 00000000000..022ebc32239 --- /dev/null +++ b/src/compiler/nir/tests/lower_returns_tests.cpp @@ -0,0 +1,213 @@ +/* + * Copyright © 2020 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ +#include +#include "nir.h" +#include "nir_builder.h" + +class nir_opt_lower_returns_test : public ::testing::Test { +protected: + nir_opt_lower_returns_test(); + ~nir_opt_lower_returns_test(); + + nir_builder bld; + + nir_ssa_def *in_def; +}; + +nir_opt_lower_returns_test::nir_opt_lower_returns_test() +{ + glsl_type_singleton_init_or_ref(); + + static const nir_shader_compiler_options options = { }; + nir_builder_init_simple_shader(&bld, NULL, MESA_SHADER_VERTEX, &options); + + nir_variable *var = nir_variable_create(bld.shader, nir_var_shader_in, glsl_int_type(), "in"); + in_def = nir_load_var(&bld, var); +} + +nir_opt_lower_returns_test::~nir_opt_lower_returns_test() +{ + ralloc_free(bld.shader); + glsl_type_singleton_decref(); +} + +nir_phi_instr *create_one_source_phi(nir_shader *shader, nir_block *pred, + nir_ssa_def *def) +{ + nir_phi_instr *phi = nir_phi_instr_create(shader); + + nir_phi_src *phi_src; + phi_src = ralloc(phi, nir_phi_src); + phi_src->pred = pred; + phi_src->src = nir_src_for_ssa(def); + exec_list_push_tail(&phi->srcs, &phi_src->node); + + nir_ssa_dest_init(&phi->instr, &phi->dest, + def->num_components, def->bit_size, NULL); + + return phi; +} + +TEST_F(nir_opt_lower_returns_test, phis_after_loop) +{ + /* Test that after lowering of "return" the phis in block_5 + * have two sources, because block_2 will have block_5 + * as a successor. + * + * block block_0: + * loop { + * block block_1: + * if ssa_2 { + * block block_2: + * return + * // succs: block_6 + * } else { + * block block_3: + * break; + * // succs: block_5 + * } + * block block_4: + * } + * block block_5: + * // preds: block_3 + * vec1 32 ssa_4 = phi block_3: ssa_1 + * vec1 32 ssa_5 = phi block_3: ssa_1 + * // succs: block_6 + * block block_6: + */ + + nir_loop *loop = nir_push_loop(&bld); + + nir_ssa_def *one = nir_imm_int(&bld, 1); + + nir_ssa_def *cmp_result = nir_ieq(&bld, in_def, one); + nir_if *nif = nir_push_if(&bld, cmp_result); + + nir_jump(&bld, nir_jump_return); + + nir_push_else(&bld, NULL); + + nir_jump(&bld, nir_jump_break); + + nir_pop_if(&bld, NULL); + + nir_block *else_block = nir_if_last_else_block(nif); + + nir_pop_loop(&bld, loop); + + bld.cursor = nir_after_cf_node_and_phis(&loop->cf_node); + + nir_phi_instr *const phi_1 = + create_one_source_phi(bld.shader, else_block, one); + nir_builder_instr_insert(&bld, &phi_1->instr); + + nir_phi_instr *const phi_2 = + create_one_source_phi(bld.shader, else_block, one); + nir_builder_instr_insert(&bld, &phi_2->instr); + + ASSERT_TRUE(nir_lower_returns(bld.shader)); + EXPECT_EQ(phi_1->srcs.length(), 2); + EXPECT_EQ(phi_2->srcs.length(), 2); + + nir_validate_shader(bld.shader, NULL); +} + +TEST_F(nir_opt_lower_returns_test, phis_after_outer_loop) +{ + /* Test that after lowering of "return" the phis in block_7 + * have two sources, because block_6 will have a conditional break + * inserted, which will add a new predcessor to block_7. + * + * block block_0: + * loop { + * block block_1: + * loop { + * block block_2: + * if ssa_2 { + * block block_3: + * return + * // succs: block_8 + * } else { + * block block_4: + * break; + * // succs: block_6 + * } + * block block_5: + * } + * block block_6: + * break; + * // succs: block_7 + * } + * block block_7: + * // preds: block_6 + * vec1 32 ssa_4 = phi block_6: ssa_1 + * vec1 32 ssa_5 = phi block_6: ssa_1 + * // succs: block_8 + * block block_8: + */ + + nir_loop *loop_outer = nir_push_loop(&bld); + + bld.cursor = nir_after_cf_list(&loop_outer->body); + + nir_loop *loop_inner = nir_push_loop(&bld); + + bld.cursor = nir_after_cf_list(&loop_inner->body); + + nir_ssa_def *one = nir_imm_int(&bld, 1); + + nir_ssa_def *cmp_result = nir_ieq(&bld, in_def, one); + nir_push_if(&bld, cmp_result); + + nir_jump(&bld, nir_jump_return); + + nir_push_else(&bld, NULL); + + nir_jump(&bld, nir_jump_break); + + nir_pop_if(&bld, NULL); + + nir_pop_loop(&bld, loop_inner); + + bld.cursor = nir_after_cf_node_and_phis(&loop_inner->cf_node); + + nir_jump(&bld, nir_jump_break); + + nir_pop_loop(&bld, loop_outer); + + bld.cursor = nir_after_cf_node_and_phis(&loop_outer->cf_node); + + nir_phi_instr *const phi_1 = + create_one_source_phi(bld.shader, nir_loop_last_block(loop_outer), one); + nir_builder_instr_insert(&bld, &phi_1->instr); + + nir_phi_instr *const phi_2 = + create_one_source_phi(bld.shader, nir_loop_last_block(loop_outer), one); + nir_builder_instr_insert(&bld, &phi_2->instr); + + ASSERT_TRUE(nir_lower_returns(bld.shader)); + EXPECT_EQ(phi_1->srcs.length(), 2); + EXPECT_EQ(phi_2->srcs.length(), 2); + + nir_validate_shader(bld.shader, NULL); +}