Module: Mesa Branch: main Commit: bdbf873b0f25d0b9cf3de0f249f8f6faad4ac9a8 URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=bdbf873b0f25d0b9cf3de0f249f8f6faad4ac9a8
Author: Daniel Schürmann <dan...@schuermann.dev> Date: Tue Aug 8 16:48:49 2023 +0200 nir: remove redundant passes from nir_opt_if() These are now covered by nir_opt_loop(): - opt_if_loop_last_continue() - opt_merge_breaks() - opt_if_loop_terminator() Reviewed-by: Konstantin Seurer <konstantin.seu...@gmail.com> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24940> --- src/amd/vulkan/radv_shader.c | 3 +- src/asahi/clc/asahi_clc.c | 3 +- src/compiler/nir/nir.h | 5 +- src/compiler/nir/nir_opt_if.c | 333 -------------------------- src/gallium/auxiliary/nir/nir_to_tgsi.c | 2 +- src/gallium/drivers/i915/i915_screen.c | 4 +- src/gallium/drivers/r300/compiler/r300_nir.c | 2 +- src/gallium/drivers/radeonsi/si_shader_nir.c | 1 - src/gallium/frontends/lavapipe/lvp_pipeline.c | 2 +- src/gallium/frontends/rusticl/core/kernel.rs | 3 +- src/microsoft/clc/clc_compiler.c | 2 +- src/microsoft/compiler/nir_to_dxil.c | 2 +- src/nouveau/vulkan/nvk_codegen.c | 3 +- 13 files changed, 12 insertions(+), 353 deletions(-) diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c index d987307cc9b..a24933f0db0 100644 --- a/src/amd/vulkan/radv_shader.c +++ b/src/amd/vulkan/radv_shader.c @@ -210,8 +210,7 @@ radv_optimize_nir(struct nir_shader *shader, bool optimize_conservatively) NIR_LOOP_PASS(progress, skip, shader, nir_opt_remove_phis); NIR_LOOP_PASS(progress, skip, shader, nir_opt_dce); } - NIR_LOOP_PASS_NOT_IDEMPOTENT(progress, skip, shader, nir_opt_if, - nir_opt_if_aggressive_last_continue | nir_opt_if_optimize_phi_true_false); + NIR_LOOP_PASS_NOT_IDEMPOTENT(progress, skip, shader, nir_opt_if, nir_opt_if_optimize_phi_true_false); NIR_LOOP_PASS(progress, skip, shader, nir_opt_dead_cf); NIR_LOOP_PASS(progress, skip, shader, nir_opt_cse); NIR_LOOP_PASS(progress, skip, shader, nir_opt_peephole_select, 8, true, true); diff --git a/src/asahi/clc/asahi_clc.c b/src/asahi/clc/asahi_clc.c index e34fe798b81..e90e898c06a 100644 --- a/src/asahi/clc/asahi_clc.c +++ b/src/asahi/clc/asahi_clc.c @@ -17,7 +17,6 @@ #include "nir_builder.h" #include "nir_serialize.h" -#include <errno.h> #include <fcntl.h> #include <getopt.h> #include <inttypes.h> @@ -214,7 +213,7 @@ compile(void *memctx, const uint32_t *spirv, size_t spirv_size) */ NIR_PASS_V(nir, nir_lower_convert_alu_types, NULL); - NIR_PASS_V(nir, nir_opt_if, nir_opt_if_aggressive_last_continue); + NIR_PASS_V(nir, nir_opt_if, 0); NIR_PASS_V(nir, nir_opt_idiv_const, 16); optimize(nir); diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index 0c729558cc7..0aac0382480 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -6301,9 +6301,8 @@ bool nir_opt_gcm(nir_shader *shader, bool value_number); bool nir_opt_idiv_const(nir_shader *shader, unsigned min_bit_size); typedef enum { - nir_opt_if_aggressive_last_continue = (1 << 0), - nir_opt_if_optimize_phi_true_false = (1 << 1), - nir_opt_if_avoid_64bit_phis = (1 << 2), + nir_opt_if_optimize_phi_true_false = (1 << 0), + nir_opt_if_avoid_64bit_phis = (1 << 1), } nir_opt_if_options; bool nir_opt_if(nir_shader *shader, nir_opt_if_options options); diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index dd0fc7f0be1..78ce28859a0 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -697,125 +697,6 @@ is_block_empty(nir_block *block) exec_list_is_empty(&block->instr_list); } -static bool -nir_block_ends_in_continue(nir_block *block) -{ - if (exec_list_is_empty(&block->instr_list)) - return false; - - nir_instr *instr = nir_block_last_instr(block); - return instr->type == nir_instr_type_jump && - nir_instr_as_jump(instr)->type == nir_jump_continue; -} - -/** - * This optimization turns: - * - * loop { - * ... - * if (cond) { - * do_work_1(); - * continue; - * } else { - * } - * do_work_2(); - * } - * - * into: - * - * loop { - * ... - * if (cond) { - * do_work_1(); - * continue; - * } else { - * do_work_2(); - * } - * } - * - * The continue should then be removed by nir_opt_trivial_continues() and the - * loop can potentially be unrolled. - * - * 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, bool aggressive_last_continue) -{ - nir_if *nif = NULL; - 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) { - 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); - - then_ends_in_continue = nir_block_ends_in_continue(then_block); - else_ends_in_continue = nir_block_ends_in_continue(else_block); - - /* 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; - - /* If continue found stop scanning and attempt optimisation, or - */ - if (then_ends_in_continue || else_ends_in_continue || - !aggressive_last_continue) - break; - } - - if_node = nir_cf_node_prev(if_node); - } - - /* If we didn't find an if to optimise return */ - if (!nif || (!then_ends_in_continue && !else_ends_in_continue)) - return false; - - /* 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; - - /* If there are single-source phis in the last block, - * get rid of them first - */ - nir_opt_remove_phis_block(last_block); - - /* Move the last block of the loop inside the last if-statement */ - 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_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_reg_intrinsics_to_ssa_impl() every - * time an if opt makes progress we leave nir_opt_trivial_continues() to - * remove the continue now that the end of the loop has been simplified. - */ - - return true; -} - /* Walk all the phis in the block immediately following the if statement and * swap the blocks. */ @@ -947,210 +828,6 @@ opt_if_phi_is_condition(nir_builder *b, nir_if *nif) return progress; } -/** - * This optimization tries to merge two break statements into a single break. - * For this purpose, it checks if both branch legs end in a break or - * if one branch leg ends in a break, and the other one does so after the - * branch. - * - * This optimization turns - * - * loop { - * ... - * if (cond) { - * do_work_1(); - * break; - * } else { - * do_work_2(); - * break; - * } - * } - * - * into: - * - * loop { - * ... - * if (cond) { - * do_work_1(); - * } else { - * do_work_2(); - * } - * break; - * } - * - * but also situations like - * - * loop { - * ... - * if (cond1) { - * if (cond2) { - * do_work_1(); - * break; - * } else { - * do_work_2(); - * } - * do_work_3(); - * break; - * } else { - * ... - * } - * } - * - * into: - * - * loop { - * ... - * if (cond1) { - * if (cond2) { - * do_work_1(); - * } else { - * do_work_2(); - * do_work_3(); - * } - * break; - * } else { - * ... - * } - * } - */ -static bool -opt_merge_breaks(nir_if *nif) -{ - nir_block *last_then = nir_if_last_then_block(nif); - nir_block *last_else = nir_if_last_else_block(nif); - bool then_break = nir_block_ends_in_break(last_then); - bool else_break = nir_block_ends_in_break(last_else); - - /* If both branch legs end in a break, merge the break after the branch */ - if (then_break && else_break) { - nir_block *after_if = nir_cf_node_cf_tree_next(&nif->cf_node); - /* Make sure that the successor is empty. - * If not we let nir_opt_dead_cf() clean it up first. - */ - if (!is_block_empty(after_if)) - return false; - - nir_lower_phis_to_regs_block(last_then->successors[0]); - nir_instr_remove_v(nir_block_last_instr(last_then)); - nir_instr *jump = nir_block_last_instr(last_else); - nir_instr_remove_v(jump); - nir_instr_insert(nir_after_block(after_if), jump); - return true; - } - - /* Single break: If there's a break after the branch and the non-breaking - * side of the if falls through to it, then hoist that code after up into - * the if and leave just a single break there. - */ - if (then_break || else_break) { - - /* At least one branch leg must fall-through */ - if (nir_block_ends_in_jump(last_then) && nir_block_ends_in_jump(last_else)) - return false; - - /* Check if there is a single break after the IF */ - nir_cf_node *first = nir_cf_node_next(&nif->cf_node); - nir_cf_node *last = first; - while (!nir_cf_node_is_last(last)) { - if (contains_other_jump(last, NULL)) - return false; - last = nir_cf_node_next(last); - } - - assert(last->type == nir_cf_node_block); - if (!nir_block_ends_in_break(nir_cf_node_as_block(last))) - return false; - - /* Hoist the code from after the IF into the falling-through branch leg */ - nir_opt_remove_phis_block(nir_cf_node_as_block(first)); - nir_block *break_block = then_break ? last_then : last_else; - nir_lower_phis_to_regs_block(break_block->successors[0]); - - nir_cf_list tmp; - nir_cf_extract(&tmp, nir_before_cf_node(first), - nir_after_block_before_jump(nir_cf_node_as_block(last))); - if (then_break) - nir_cf_reinsert(&tmp, nir_after_block(last_else)); - else - nir_cf_reinsert(&tmp, nir_after_block(last_then)); - - nir_instr_remove_v(nir_block_last_instr(break_block)); - return true; - } - - return false; -} - -/** - * This optimization simplifies potential loop terminators which then allows - * other passes such as opt_if_simplification() and loop unrolling to progress - * further: - * - * if (cond) { - * ... then block instructions ... - * } else { - * ... - * break; - * } - * - * into: - * - * if (cond) { - * } else { - * ... - * break; - * } - * ... then block instructions ... - */ -static bool -opt_if_loop_terminator(nir_if *nif) -{ - nir_block *break_blk = NULL; - nir_block *continue_from_blk = NULL; - bool continue_from_then = true; - - nir_block *last_then = nir_if_last_then_block(nif); - nir_block *last_else = nir_if_last_else_block(nif); - - if (nir_block_ends_in_break(last_then)) { - break_blk = last_then; - continue_from_blk = last_else; - continue_from_then = false; - } else if (nir_block_ends_in_break(last_else)) { - break_blk = last_else; - continue_from_blk = last_then; - } - - /* Continue if the if-statement contained no jumps at all */ - if (!break_blk) - return false; - - /* If the continue from block is empty then return as there is nothing to - * move. - */ - nir_block *first_continue_from_blk = continue_from_then ? nir_if_first_then_block(nif) : nir_if_first_else_block(nif); - if (is_block_empty(first_continue_from_blk)) - return false; - - 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), - nir_after_block(continue_from_blk)); - nir_cf_reinsert(&tmp, nir_after_cf_node(&nif->cf_node)); - - return true; -} - static bool evaluate_if_condition(nir_if *nif, nir_cursor cursor, bool *value) { @@ -1535,7 +1212,6 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list, options); progress |= opt_if_cf_list(b, &nif->else_list, options); - progress |= opt_if_loop_terminator(nif); progress |= opt_if_merge(nif); progress |= opt_if_simplification(b, nif); if (options & nir_opt_if_optimize_phi_true_false) @@ -1549,8 +1225,6 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list, progress |= opt_if_cf_list(b, &loop->body, options); progress |= opt_simplify_bcsel_of_phi(b, loop); - progress |= opt_if_loop_last_continue(loop, - options & nir_opt_if_aggressive_last_continue); break; } @@ -1579,13 +1253,6 @@ opt_if_regs_cf_list(struct exec_list *cf_list) nir_if *nif = nir_cf_node_as_if(cf_node); progress |= opt_if_regs_cf_list(&nif->then_list); progress |= opt_if_regs_cf_list(&nif->else_list); - if (opt_merge_breaks(nif)) { - /* This optimization might move blocks - * from after the NIF into the NIF */ - progress = true; - opt_if_regs_cf_list(&nif->then_list); - opt_if_regs_cf_list(&nif->else_list); - } break; } diff --git a/src/gallium/auxiliary/nir/nir_to_tgsi.c b/src/gallium/auxiliary/nir/nir_to_tgsi.c index 34c370edff7..1c603e4cbf7 100644 --- a/src/gallium/auxiliary/nir/nir_to_tgsi.c +++ b/src/gallium/auxiliary/nir/nir_to_tgsi.c @@ -3331,7 +3331,7 @@ ntt_optimize_nir(struct nir_shader *s, struct pipe_screen *screen, NIR_PASS(progress, s, nir_opt_copy_prop_vars); NIR_PASS(progress, s, nir_opt_dead_write_vars); - NIR_PASS(progress, s, nir_opt_if, nir_opt_if_aggressive_last_continue | nir_opt_if_optimize_phi_true_false); + NIR_PASS(progress, s, nir_opt_if, nir_opt_if_optimize_phi_true_false); NIR_PASS(progress, s, nir_opt_peephole_select, control_flow_depth == 0 ? ~0 : 8, true, true); NIR_PASS(progress, s, nir_opt_algebraic); diff --git a/src/gallium/drivers/i915/i915_screen.c b/src/gallium/drivers/i915/i915_screen.c index cfd94261370..82b7c987518 100644 --- a/src/gallium/drivers/i915/i915_screen.c +++ b/src/gallium/drivers/i915/i915_screen.c @@ -205,9 +205,7 @@ i915_optimize_nir(struct nir_shader *s) NIR_PASS(progress, s, nir_opt_dead_cf); NIR_PASS(progress, s, nir_opt_cse); NIR_PASS(progress, s, nir_opt_find_array_copies); - NIR_PASS(progress, s, nir_opt_if, - nir_opt_if_aggressive_last_continue | - nir_opt_if_optimize_phi_true_false); + NIR_PASS(progress, s, nir_opt_if, nir_opt_if_optimize_phi_true_false); NIR_PASS(progress, s, nir_opt_peephole_select, ~0 /* flatten all IFs. */, true, true); NIR_PASS(progress, s, nir_opt_algebraic); diff --git a/src/gallium/drivers/r300/compiler/r300_nir.c b/src/gallium/drivers/r300/compiler/r300_nir.c index d82fc7c2865..bc52a218282 100644 --- a/src/gallium/drivers/r300/compiler/r300_nir.c +++ b/src/gallium/drivers/r300/compiler/r300_nir.c @@ -85,7 +85,7 @@ r300_optimize_nir(struct nir_shader *s, struct pipe_screen *screen) NIR_PASS(progress, s, nir_opt_copy_prop_vars); NIR_PASS(progress, s, nir_opt_dead_write_vars); - NIR_PASS(progress, s, nir_opt_if, nir_opt_if_aggressive_last_continue | nir_opt_if_optimize_phi_true_false); + NIR_PASS(progress, s, nir_opt_if, nir_opt_if_optimize_phi_true_false); NIR_PASS(progress, s, nir_opt_peephole_select, is_r500 ? 8 : ~0, true, true); NIR_PASS(progress, s, nir_opt_algebraic); NIR_PASS(progress, s, nir_opt_constant_folding); diff --git a/src/gallium/drivers/radeonsi/si_shader_nir.c b/src/gallium/drivers/radeonsi/si_shader_nir.c index d17de169459..027d9c410b8 100644 --- a/src/gallium/drivers/radeonsi/si_shader_nir.c +++ b/src/gallium/drivers/radeonsi/si_shader_nir.c @@ -98,7 +98,6 @@ void si_nir_opts(struct si_screen *sscreen, struct nir_shader *nir, bool first) NIR_PASS(progress, nir, nir_opt_dce); /* nir_opt_if_optimize_phi_true_false is disabled on LLVM14 (#6976) */ NIR_PASS(lower_phis_to_scalar, nir, nir_opt_if, - nir_opt_if_aggressive_last_continue | nir_opt_if_optimize_phi_true_false); NIR_PASS(progress, nir, nir_opt_dead_cf); diff --git a/src/gallium/frontends/lavapipe/lvp_pipeline.c b/src/gallium/frontends/lavapipe/lvp_pipeline.c index e5dcb9c4275..528b66b908b 100644 --- a/src/gallium/frontends/lavapipe/lvp_pipeline.c +++ b/src/gallium/frontends/lavapipe/lvp_pipeline.c @@ -247,7 +247,7 @@ optimize(nir_shader *nir) NIR_PASS(progress, nir, nir_opt_dce); NIR_PASS(progress, nir, nir_opt_remove_phis); } - NIR_PASS(progress, nir, nir_opt_if, nir_opt_if_aggressive_last_continue | nir_opt_if_optimize_phi_true_false); + NIR_PASS(progress, nir, nir_opt_if, nir_opt_if_optimize_phi_true_false); NIR_PASS(progress, nir, nir_opt_dead_cf); NIR_PASS(progress, nir, nir_opt_conditional_discard); NIR_PASS(progress, nir, nir_opt_remove_phis); diff --git a/src/gallium/frontends/rusticl/core/kernel.rs b/src/gallium/frontends/rusticl/core/kernel.rs index 958539aeff9..8c5ab582d12 100644 --- a/src/gallium/frontends/rusticl/core/kernel.rs +++ b/src/gallium/frontends/rusticl/core/kernel.rs @@ -356,8 +356,7 @@ fn opt_nir(nir: &mut NirShader, dev: &Device) { progress |= nir_pass!( nir, nir_opt_if, - nir_opt_if_options::nir_opt_if_aggressive_last_continue - | nir_opt_if_options::nir_opt_if_optimize_phi_true_false, + nir_opt_if_options::nir_opt_if_optimize_phi_true_false, ); progress |= nir_pass!(nir, nir_opt_dead_cf); progress |= nir_pass!(nir, nir_opt_remove_phis); diff --git a/src/microsoft/clc/clc_compiler.c b/src/microsoft/clc/clc_compiler.c index 8b32d1d55d8..b3ef8b33aab 100644 --- a/src/microsoft/clc/clc_compiler.c +++ b/src/microsoft/clc/clc_compiler.c @@ -749,7 +749,7 @@ clc_spirv_to_dxil(struct clc_libclc *lib, NIR_PASS(progress, nir, nir_lower_var_copies); NIR_PASS(progress, nir, nir_lower_vars_to_ssa); NIR_PASS(progress, nir, nir_opt_algebraic); - NIR_PASS(progress, nir, nir_opt_if, nir_opt_if_aggressive_last_continue | nir_opt_if_optimize_phi_true_false); + NIR_PASS(progress, nir, nir_opt_if, nir_opt_if_optimize_phi_true_false); NIR_PASS(progress, nir, nir_opt_dead_cf); NIR_PASS(progress, nir, nir_opt_remove_phis); NIR_PASS(progress, nir, nir_opt_peephole_select, 8, true, true); diff --git a/src/microsoft/compiler/nir_to_dxil.c b/src/microsoft/compiler/nir_to_dxil.c index 49554168788..e67a40b14bd 100644 --- a/src/microsoft/compiler/nir_to_dxil.c +++ b/src/microsoft/compiler/nir_to_dxil.c @@ -6232,7 +6232,7 @@ optimize_nir(struct nir_shader *s, const struct nir_to_dxil_options *opts) NIR_PASS(progress, s, nir_opt_remove_phis); NIR_PASS(progress, s, nir_opt_dce); NIR_PASS(progress, s, nir_opt_if, - nir_opt_if_aggressive_last_continue | nir_opt_if_optimize_phi_true_false | nir_opt_if_avoid_64bit_phis); + nir_opt_if_optimize_phi_true_false | nir_opt_if_avoid_64bit_phis); NIR_PASS(progress, s, nir_opt_dead_cf); NIR_PASS(progress, s, nir_opt_cse); NIR_PASS(progress, s, nir_opt_peephole_select, 8, true, true); diff --git a/src/nouveau/vulkan/nvk_codegen.c b/src/nouveau/vulkan/nvk_codegen.c index 644a19428e0..e4b6423c26b 100644 --- a/src/nouveau/vulkan/nvk_codegen.c +++ b/src/nouveau/vulkan/nvk_codegen.c @@ -155,8 +155,7 @@ nvk_cg_optimize_nir(nir_shader *nir) NIR_PASS(progress, nir, nir_opt_remove_phis); NIR_PASS(progress, nir, nir_opt_dce); } - NIR_PASS(progress, nir, nir_opt_if, - nir_opt_if_aggressive_last_continue | nir_opt_if_optimize_phi_true_false); + NIR_PASS(progress, nir, nir_opt_if, nir_opt_if_optimize_phi_true_false); NIR_PASS(progress, nir, nir_opt_dead_cf); NIR_PASS(progress, nir, nir_opt_cse); /*
