static bool
phi_has_constant_from_outside_and_one_from_inside_loop(nir_phi_instr *phi,
const nir_block *entry_block,
- uint32_t *entry_val,
- uint32_t *continue_val)
+ bool *entry_val,
+ bool *continue_val)
{
/* We already know we have exactly one continue */
assert(exec_list_length(&phi->srcs) == 2);
- *entry_val = 0;
- *continue_val = 0;
+ *entry_val = false;
+ *continue_val = false;
nir_foreach_phi_src(src, phi) {
- assert(src->src.is_ssa);
- nir_const_value *const_src = nir_src_as_const_value(src->src);
- if (!const_src)
- return false;
+ if (!nir_src_is_const(src->src))
+ return false;
if (src->pred != entry_block) {
- *continue_val = const_src->u32[0];
+ *continue_val = nir_src_as_bool(src->src);
} else {
- *entry_val = const_src->u32[0];
+ *entry_val = nir_src_as_bool(src->src);
}
}
if (cond->parent_instr->block != header_block)
return false;
- uint32_t entry_val = 0, continue_val = 0;
+ bool entry_val = false, continue_val = false;
if (!phi_has_constant_from_outside_and_one_from_inside_loop(cond_phi,
prev_block,
&entry_val,
nir_cf_reinsert(&header,
nir_after_block_before_jump(find_continue_block(loop)));
+ bool continue_list_jumps =
+ nir_block_ends_in_jump(exec_node_data(nir_block,
+ exec_list_get_tail(continue_list),
+ cf_node.node));
+
nir_cf_extract(&tmp, nir_before_cf_list(continue_list),
nir_after_cf_list(continue_list));
- /* Get continue block again as the previous reinsert might have removed the block. */
+ /* Get continue block again as the previous reinsert might have removed the
+ * block. Also, if both the continue list and the continue block ends in
+ * jump instructions, removes the jump from the latter, as it will not be
+ * executed if we insert the continue list before it. */
+
+ nir_block *continue_block = find_continue_block(loop);
+
+ if (continue_list_jumps) {
+ nir_instr *last_instr = nir_block_last_instr(continue_block);
+ if (last_instr && last_instr->type == nir_instr_type_jump)
+ nir_instr_remove(last_instr);
+ }
+
nir_cf_reinsert(&tmp,
- nir_after_block_before_jump(find_continue_block(loop)));
+ nir_after_block_before_jump(continue_block));
nir_cf_node_remove(&nif->cf_node);
case nir_op_flt32:
case nir_op_fge32:
case nir_op_feq32:
- case nir_op_fne32:
+ case nir_op_fneu32:
case nir_op_ilt32:
case nir_op_ult32:
case nir_op_ige32:
alu_instr_is_type_conversion(const nir_alu_instr *alu)
{
return nir_op_infos[alu->op].num_inputs == 1 &&
- nir_alu_type_get_base_type(nir_op_infos[alu->op].output_type) !=
- nir_alu_type_get_base_type(nir_op_infos[alu->op].input_types[0]);
+ nir_op_infos[alu->op].output_type != nir_op_infos[alu->op].input_types[0];
}
/**
*
* - At least one source of the instruction is a phi node from the header block.
*
- * and either this rule
- *
- * - 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 or undef from the block before the loop.
*
- * - 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
*
* 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)
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}, nir_op_imov, and nir_op_fmov 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 ||
- alu->op == nir_op_vec3 ||
- alu->op == nir_op_vec4 ||
- alu->op == nir_op_imov ||
- alu->op == nir_op_fmov ||
+ if (nir_op_is_vec(alu->op) ||
alu_instr_is_comparison(alu) ||
alu_instr_is_type_conversion(alu))
continue;
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
* bcsel that must come before any break.
*
* For more details, see
- * https://gitlab.freedesktop.org/mesa/mesa/merge_requests/170#note_110305
+ * https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/170#note_110305
*/
nir_foreach_instr_safe(instr, header_block) {
if (instr->type != nir_instr_type_alu)
nir_phi_instr *const cond_phi =
nir_instr_as_phi(bcsel->src[0].src.ssa->parent_instr);
- uint32_t entry_val = 0, continue_val = 0;
+ bool entry_val = false, continue_val = false;
if (!phi_has_constant_from_outside_and_one_from_inside_loop(cond_phi,
prev_block,
&entry_val,
* The continue should then be removed by nir_opt_trivial_continues() and the
* loop can potentially be unrolled.
*
- * Note: do_work_2() is only ever blocks and nested loops. We could also nest
- * other if-statments in the branch which would allow further continues to
- * be removed. However in practice this can result in increased register
- * pressure.
+ * Note: Unless the function param aggressive_last_continue==true do_work_2()
+ * is only ever blocks and nested loops. We avoid nesting other if-statments
+ * in the branch as this can result in increased register pressure, and in
+ * the i965 driver it causes a large amount of spilling in shader-db.
+ * For RADV however nesting these if-statements allows further continues to be
+ * remove and provides a significant FPS boost in Doom, which is why we have
+ * opted for this special bool to enable more aggresive optimisations.
+ * TODO: The GCM pass solves most of the spilling regressions in i965, if it
+ * is ever enabled we should consider removing the aggressive_last_continue
+ * param.
*/
static bool
-opt_if_loop_last_continue(nir_loop *loop)
+opt_if_loop_last_continue(nir_loop *loop, bool aggressive_last_continue)
{
- /* Get the last if-stament in the loop */
+ nir_if *nif;
+ bool then_ends_in_continue = false;
+ bool else_ends_in_continue = false;
+
+ /* Scan the control flow of the loop from the last to the first node
+ * looking for an if-statement we can optimise.
+ */
nir_block *last_block = nir_loop_last_block(loop);
nir_cf_node *if_node = nir_cf_node_prev(&last_block->cf_node);
while (if_node) {
- if (if_node->type == nir_cf_node_if)
- break;
+ if (if_node->type == nir_cf_node_if) {
+ nif = nir_cf_node_as_if(if_node);
+ nir_block *then_block = nir_if_last_then_block(nif);
+ nir_block *else_block = nir_if_last_else_block(nif);
- if_node = nir_cf_node_prev(if_node);
- }
-
- if (!if_node || if_node->type != nir_cf_node_if)
- return false;
+ then_ends_in_continue = nir_block_ends_in_continue(then_block);
+ else_ends_in_continue = nir_block_ends_in_continue(else_block);
- nir_if *nif = nir_cf_node_as_if(if_node);
- nir_block *then_block = nir_if_last_then_block(nif);
- nir_block *else_block = nir_if_last_else_block(nif);
+ /* If both branches end in a jump do nothing, this should be handled
+ * by nir_opt_dead_cf().
+ */
+ if ((then_ends_in_continue || nir_block_ends_in_break(then_block)) &&
+ (else_ends_in_continue || nir_block_ends_in_break(else_block)))
+ return false;
- bool then_ends_in_continue = nir_block_ends_in_continue(then_block);
- bool else_ends_in_continue = nir_block_ends_in_continue(else_block);
+ /* If continue found stop scanning and attempt optimisation, or
+ */
+ if (then_ends_in_continue || else_ends_in_continue ||
+ !aggressive_last_continue)
+ break;
+ }
- /* If both branches end in a continue do nothing, this should be handled
- * by nir_opt_dead_cf().
- */
- if (then_ends_in_continue && else_ends_in_continue)
- return false;
+ if_node = nir_cf_node_prev(if_node);
+ }
+ /* If we didn't find an if to optimise return */
if (!then_ends_in_continue && !else_ends_in_continue)
return false;
- /* if the block after the if/else is empty we bail, otherwise we might end
- * up looping forever
- */
+ /* If there is nothing after the if-statement we bail */
if (&nif->cf_node == nir_cf_node_prev(&last_block->cf_node) &&
exec_list_is_empty(&last_block->instr_list))
return false;
nir_cf_list tmp;
nir_cf_extract(&tmp, nir_after_cf_node(if_node),
nir_after_block(last_block));
- if (then_ends_in_continue) {
- nir_cursor last_blk_cursor = nir_after_cf_list(&nif->else_list);
- nir_cf_reinsert(&tmp,
- nir_after_block_before_jump(last_blk_cursor.block));
- } else {
- nir_cursor last_blk_cursor = nir_after_cf_list(&nif->then_list);
- nir_cf_reinsert(&tmp,
- nir_after_block_before_jump(last_blk_cursor.block));
- }
+ if (then_ends_in_continue)
+ nir_cf_reinsert(&tmp, nir_after_cf_list(&nif->else_list));
+ else
+ nir_cf_reinsert(&tmp, nir_after_cf_list(&nif->then_list));
/* In order to avoid running nir_lower_regs_to_ssa_impl() every time an if
* opt makes progress we leave nir_opt_trivial_continues() to remove the
nir_block *then_block = nir_if_last_then_block(nif);
nir_block *else_block = nir_if_last_else_block(nif);
+ if (nir_block_ends_in_jump(else_block)) {
+ /* Even though this if statement has a jump on one side, we may still have
+ * phis afterwards. Single-source phis can be produced by loop unrolling
+ * or dead control-flow passes and are perfectly legal. Run a quick phi
+ * removal on the block after the if to clean up any such phis.
+ */
+ nir_block *const next_block =
+ nir_cf_node_as_block(nir_cf_node_next(&nif->cf_node));
+ nir_opt_remove_phis_block(next_block);
+ }
+
rewrite_phi_predecessor_blocks(nif, then_block, else_block, else_block,
then_block);
if (is_block_empty(first_continue_from_blk))
return false;
- if (!nir_is_trivial_loop_if(nif, break_blk))
+ if (nir_block_ends_in_jump(continue_from_blk))
return false;
+ /* Even though this if statement has a jump on one side, we may still have
+ * phis afterwards. Single-source phis can be produced by loop unrolling
+ * or dead control-flow passes and are perfectly legal. Run a quick phi
+ * removal on the block after the if to clean up any such phis.
+ */
+ nir_opt_remove_phis_block(nir_cf_node_as_block(nir_cf_node_next(&nif->cf_node)));
+
/* Finally, move the continue from branch after the if-statement. */
nir_cf_list tmp;
nir_cf_extract(&tmp, nir_before_block(first_continue_from_blk),
}
static bool
-opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
+opt_if_cf_list(nir_builder *b, struct exec_list *cf_list,
+ bool aggressive_last_continue)
{
bool progress = false;
foreach_list_typed(nir_cf_node, cf_node, node, cf_list) {
case nir_cf_node_if: {
nir_if *nif = nir_cf_node_as_if(cf_node);
- progress |= opt_if_cf_list(b, &nif->then_list);
- progress |= opt_if_cf_list(b, &nif->else_list);
+ progress |= opt_if_cf_list(b, &nif->then_list,
+ aggressive_last_continue);
+ progress |= opt_if_cf_list(b, &nif->else_list,
+ aggressive_last_continue);
progress |= opt_if_loop_terminator(nif);
progress |= opt_if_merge(nif);
progress |= opt_if_simplification(b, nif);
case nir_cf_node_loop: {
nir_loop *loop = nir_cf_node_as_loop(cf_node);
- progress |= opt_if_cf_list(b, &loop->body);
+ progress |= opt_if_cf_list(b, &loop->body,
+ aggressive_last_continue);
progress |= opt_simplify_bcsel_of_phi(b, loop);
+ progress |= opt_if_loop_last_continue(loop,
+ aggressive_last_continue);
+ break;
+ }
+
+ case nir_cf_node_function:
+ unreachable("Invalid cf type");
+ }
+ }
+
+ return progress;
+}
+
+static bool
+opt_peel_loop_initial_if_cf_list(struct exec_list *cf_list)
+{
+ bool progress = false;
+ foreach_list_typed(nir_cf_node, cf_node, node, cf_list) {
+ switch (cf_node->type) {
+ case nir_cf_node_block:
+ break;
+
+ case nir_cf_node_if: {
+ nir_if *nif = nir_cf_node_as_if(cf_node);
+ progress |= opt_peel_loop_initial_if_cf_list(&nif->then_list);
+ progress |= opt_peel_loop_initial_if_cf_list(&nif->else_list);
+ break;
+ }
+
+ case nir_cf_node_loop: {
+ nir_loop *loop = nir_cf_node_as_loop(cf_node);
+ progress |= opt_peel_loop_initial_if_cf_list(&loop->body);
progress |= opt_peel_loop_initial_if(loop);
- progress |= opt_if_loop_last_continue(loop);
break;
}
}
bool
-nir_opt_if(nir_shader *shader)
+nir_opt_if(nir_shader *shader, bool aggressive_last_continue)
{
bool progress = false;
nir_metadata_preserve(function->impl, nir_metadata_block_index |
nir_metadata_dominance);
- if (opt_if_cf_list(&b, &function->impl->body)) {
- nir_metadata_preserve(function->impl, nir_metadata_none);
+ bool preserve = true;
+
+ if (opt_if_cf_list(&b, &function->impl->body, aggressive_last_continue)) {
+ preserve = false;
+ progress = true;
+ }
+
+ if (opt_peel_loop_initial_if_cf_list(&function->impl->body)) {
+ preserve = false;
+ progress = true;
/* If that made progress, we're no longer really in SSA form. We
* need to convert registers back into SSA defs and clean up SSA defs
* that don't dominate their uses.
*/
nir_lower_regs_to_ssa_impl(function->impl);
+ }
- progress = true;
+ if (preserve) {
+ nir_metadata_preserve(function->impl, nir_metadata_none);
} else {
- #ifndef NDEBUG
- function->impl->valid_metadata &= ~nir_metadata_not_properly_reset;
- #endif
+ nir_metadata_preserve(function->impl, nir_metadata_all);
}
}
projects / mesa.git / blobdiff