diff --git a/src/amd/compiler/tests/test_isel.cpp b/src/amd/compiler/tests/test_isel.cpp index a6aae159064..d74822810bd 100644 --- a/src/amd/compiler/tests/test_isel.cpp +++ b/src/amd/compiler/tests/test_isel.cpp @@ -42,12 +42,12 @@ BEGIN_TEST(isel.interp.simple) void main() { //>> v1: %a_tmp = v_interp_p1_f32 %bx, %pm:m0 attr0.w //! v1: %a = v_interp_p2_f32 %by, %pm:m0, (kill)%a_tmp attr0.w - //! v1: %b_tmp = v_interp_p1_f32 %bx, %pm:m0 attr0.z - //! v1: %b = v_interp_p2_f32 %by, %pm:m0, (kill)%b_tmp attr0.z + //! v1: %r_tmp = v_interp_p1_f32 %bx, %pm:m0 attr0.x + //! v1: %r = v_interp_p2_f32 %by, %pm:m0, (kill)%r_tmp attr0.x //! v1: %g_tmp = v_interp_p1_f32 %bx, %pm:m0 attr0.y //! v1: %g = v_interp_p2_f32 %by, %pm:m0, (kill)%g_tmp attr0.y - //! v1: %r_tmp = v_interp_p1_f32 (kill)%bx, %pm:m0 attr0.x - //! v1: %r = v_interp_p2_f32 (kill)%by, (kill)%pm:m0, (kill)%r_tmp attr0.x + //! v1: %b_tmp = v_interp_p1_f32 (kill)%bx, %pm:m0 attr0.z + //! v1: %b = v_interp_p2_f32 (kill)%by, (kill)%pm:m0, (kill)%b_tmp attr0.z //! exp (kill)%r, (kill)%g, (kill)%b, (kill)%a mrt0 out_color = in_color; } diff --git a/src/compiler/nir/nir_opt_move.c b/src/compiler/nir/nir_opt_move.c index 8abdaf364f5..97baf4567e4 100644 --- a/src/compiler/nir/nir_opt_move.c +++ b/src/compiler/nir/nir_opt_move.c @@ -52,92 +52,72 @@ */ static bool -move_source(nir_src *src, nir_block *block, nir_instr *before, nir_move_options options) -{ - if (!src->is_ssa) - return false; - - nir_instr *src_instr = src->ssa->parent_instr; - - if (src_instr->block == block && nir_can_move_instr(src_instr, options)) { - exec_node_remove(&src_instr->node); - - if (before) - exec_node_insert_node_before(&before->node, &src_instr->node); - else - exec_list_push_tail(&block->instr_list, &src_instr->node); - - return true; - } - return false; -} - -struct source_cb_data { - bool *progress; - nir_move_options options; -}; - -static bool -move_source_cb(nir_src *src, void *data_ptr) -{ - struct source_cb_data data = *(struct source_cb_data*)data_ptr; - - nir_instr *instr = src->parent_instr; - if (move_source(src, instr->block, instr, data.options)) - *data.progress = true; - - return true; /* nir_foreach_src should keep going */ -} - -static bool -move(nir_block *block, nir_move_options options) +nir_opt_move_block(nir_block *block, nir_move_options options) { bool progress = false; + nir_instr *last_instr = nir_block_ends_in_jump(block) ? + nir_block_last_instr(block) : NULL; + const nir_if *iff = nir_block_get_following_if(block); + const nir_instr *if_cond_instr = iff ? iff->condition.parent_instr : NULL; - /* We use a simple approach: walk instructions backwards. - * - * If the instruction's source is a comparison from the same block, - * simply move it here. This may break SSA if it's used earlier in - * the block as well. However, as we walk backwards, we'll find the - * earlier use and move it again, further up. It eventually ends up - * dominating all uses again, restoring SSA form. - * - * Before walking instructions, we consider the if-condition at the - * end of the block, if one exists. It's effectively a use at the - * bottom of the block. + /* Walk the instructions backwards. + * The instructions get indexed while iterating. + * For each instruction which can be moved, find the earliest user + * and insert the instruction before it. + * If multiple instructions have the same user, + * the original order is kept. */ - nir_if *iff = nir_block_get_following_if(block); - if (iff) { - progress |= move_source(&iff->condition, block, NULL, options); - } + unsigned index = 1; + nir_foreach_instr_reverse_safe(instr, block) { + instr->index = index++; - nir_foreach_instr_reverse(instr, block) { - /* The sources of phi instructions happen after the predecessor block - * but before this block. (Yes, that's between blocks). This means - * that we don't need to move them in order for them to be correct. - * We could move them to encourage comparisons that are used in a phi to - * the end of the block, doing so correctly would make the pass - * substantially more complicated and wouldn't gain us anything since - * the phi can't use a flag value anyway. - */ + /* Check if this instruction can be moved downwards */ + if (!nir_can_move_instr(instr, options)) + continue; - if (instr->type == nir_instr_type_phi) { - /* We're going backwards so everything else is a phi too */ - break; - } else if (instr->type == nir_instr_type_alu) { - /* Walk ALU instruction sources backwards so that bcsel's boolean - * condition is processed last for when comparisons are being moved. - */ - nir_alu_instr *alu = nir_instr_as_alu(instr); - for (int i = nir_op_infos[alu->op].num_inputs - 1; i >= 0; i--) { - progress |= move_source(&alu->src[i].src, block, instr, options); - } - } else { - struct source_cb_data data; - data.progress = &progress; - data.options = options; - nir_foreach_src(instr, move_source_cb, &data); + /* Check all users in this block which is the first */ + const nir_ssa_def *def = nir_instr_ssa_def(instr); + nir_instr *first_user = instr == if_cond_instr ? NULL : last_instr; + nir_foreach_use(use, def) { + nir_instr *parent = use->parent_instr; + if (parent->type == nir_instr_type_phi || parent->block != block) + continue; + if (!first_user || parent->index > first_user->index) + first_user = parent; } + + if (first_user) { + /* Check predecessor instructions for the same index to keep the order */ + while (nir_instr_prev(first_user)->index == first_user->index) + first_user = nir_instr_prev(first_user); + + /* check if the user is already the immediate successor */ + if (nir_instr_prev(first_user) == instr) + continue; + + /* Insert the instruction before it's first user */ + exec_node_remove(&instr->node); + instr->index = first_user->index; + exec_node_insert_node_before(&first_user->node, &instr->node); + progress = true; + continue; + } + + /* No user was found in this block: + * This instruction will be moved to the end of the block. + */ + assert(nir_block_last_instr(block)->type != nir_instr_type_jump); + if (instr == nir_block_last_instr(block)) + continue; + + exec_node_remove(&instr->node); + instr->index = 0; + exec_list_push_tail(&block->instr_list, &instr->node); + + /* update last_instr */ + last_instr = instr; + + progress = true; } return progress; @@ -154,7 +134,7 @@ nir_opt_move(nir_shader *shader, nir_move_options options) bool impl_progress = false; nir_foreach_block(block, func->impl) { - if (move(block, options)) + if (nir_opt_move_block(block, options)) impl_progress = true; }