diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index e360c274bc9..4d391325050 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -309,35 +309,29 @@ alu_instr_is_type_conversion(const nir_alu_instr *alu) * * - At least one source of the instruction is a phi node from the header block. * - * and either this rule + * - The phi node selects a constant or undef from the block before the loop. * - * - The phi node selects undef from the block before the loop and a value - * from the continue block of the loop. - * - * or these two rules - * - * - The phi node selects a constant from the block before the loop. - * - * - The non-phi source of the ALU instruction comes from a block that + * - Any non-phi sources of the ALU instruction come from a block that * dominates the block before the loop. The most common failure mode for * this check is sources that are generated in the loop header block. * - * The split process moves the original ALU instruction to the bottom of the - * loop. The phi node source is replaced with the value from the phi node - * selected from the continue block (i.e., the non-undef value). A new phi - * node is added to the header block that selects either undef from the block - * before the loop or the result of the (moved) ALU instruction. + * The split process splits the original ALU instruction into two, one at the + * bottom of the loop and one at the block before the loop. The instruction + * before the loop computes the value on the first iteration, and the + * instruction at the bottom computes the value on the second, third, and so + * on. A new phi node is added to the header block that selects either the + * instruction before the loop or the one at the end, and uses of the original + * instruction are replaced by this phi. * * The splitting transforms a loop like: * - * vec1 32 ssa_7 = undefined * vec1 32 ssa_8 = load_const (0x00000001) * vec1 32 ssa_10 = load_const (0x00000000) * // succs: block_1 * loop { * block block_1: * // preds: block_0 block_4 - * vec1 32 ssa_11 = phi block_0: ssa_7, block_4: ssa_15 + * vec1 32 ssa_11 = phi block_0: ssa_10, block_4: ssa_15 * vec1 32 ssa_12 = phi block_0: ssa_1, block_4: ssa_15 * vec1 32 ssa_13 = phi block_0: ssa_10, block_4: ssa_16 * vec1 32 ssa_14 = iadd ssa_11, ssa_8 @@ -348,27 +342,22 @@ alu_instr_is_type_conversion(const nir_alu_instr *alu) * * into: * - * vec1 32 ssa_7 = undefined * vec1 32 ssa_8 = load_const (0x00000001) * vec1 32 ssa_10 = load_const (0x00000000) + * vec1 32 ssa_22 = iadd ssa_10, ssa_8 * // succs: block_1 * loop { * block block_1: * // preds: block_0 block_4 - * vec1 32 ssa_11 = phi block_0: ssa_7, block_4: ssa_15 + * vec1 32 ssa_11 = phi block_0: ssa_10, block_4: ssa_15 * vec1 32 ssa_12 = phi block_0: ssa_1, block_4: ssa_15 * vec1 32 ssa_13 = phi block_0: ssa_10, block_4: ssa_16 - * vec1 32 ssa_21 = phi block_0: sss_7, block_4: ssa_20 + * vec1 32 ssa_21 = phi block_0: ssa_22, block_4: ssa_20 * vec1 32 ssa_15 = b32csel ssa_13, ssa_21, ssa_12 * ... * vec1 32 ssa_20 = iadd ssa_15, ssa_8 * // succs: block_1 * } - * - * If the phi does not select an undef, the instruction is duplicated in the - * loop continue block (as in the undef case) and in the previous block. When - * the ALU instruction is duplicated in the previous block, the correct source - * must be selected from the phi node. */ static bool opt_split_alu_of_phi(nir_builder *b, nir_loop *loop) @@ -394,19 +383,12 @@ opt_split_alu_of_phi(nir_builder *b, nir_loop *loop) nir_alu_instr *const alu = nir_instr_as_alu(instr); - /* Most ALU ops produce an undefined result if any source is undef. - * However, operations like bcsel only produce undefined results of the - * first operand is undef. Even in the undefined case, the result - * should be one of the other two operands, so the result of the bcsel - * should never be replaced with undef. - * - * nir_op_vec{2,3,4} and nir_op_mov are excluded because they can easily - * lead to infinite optimization loops. + /* nir_op_vec{2,3,4} and nir_op_mov are excluded because they can easily + * lead to infinite optimization loops. Splitting comparisons can lead + * to loop unrolling not recognizing loop termintators, and type + * conversions also lead to regressions. */ - if (alu->op == nir_op_bcsel || - alu->op == nir_op_b32csel || - alu->op == nir_op_fcsel || - alu->op == nir_op_vec2 || + if (alu->op == nir_op_vec2 || alu->op == nir_op_vec3 || alu->op == nir_op_vec4 || alu->op == nir_op_mov || @@ -477,26 +459,9 @@ opt_split_alu_of_phi(nir_builder *b, nir_loop *loop) if (has_phi_src_from_prev_block && all_non_phi_exist_in_prev_block && (is_prev_result_undef || is_prev_result_const)) { nir_block *const continue_block = find_continue_block(loop); - nir_ssa_def *prev_value; - if (!is_prev_result_undef) { - b->cursor = nir_after_block(prev_block); - prev_value = clone_alu_and_replace_src_defs(b, alu, prev_srcs); - } else { - /* Since the undef used as the source of the original ALU - * instruction may have different number of components or - * bit size than the result of that instruction, a new - * undef must be created. - */ - nir_ssa_undef_instr *undef = - nir_ssa_undef_instr_create(b->shader, - alu->dest.dest.ssa.num_components, - alu->dest.dest.ssa.bit_size); - - nir_instr_insert_after_block(prev_block, &undef->instr); - - prev_value = &undef->def; - } + b->cursor = nir_after_block(prev_block); + nir_ssa_def *prev_value = clone_alu_and_replace_src_defs(b, alu, prev_srcs); /* Make a copy of the original ALU instruction. Replace the sources * of the new instruction that read a phi with an undef source from