From: Timothy Arceri Date: Wed, 29 Aug 2018 15:24:43 +0000 (-0500) Subject: nir: evaluate if condition uses inside the if branches X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=ef4ad7baf176fe1820c7ec771b8c801efcbc4846;p=mesa.git nir: evaluate if condition uses inside the if branches Since we know what side of the branch we ended up on we can just replace the use with a constant. All the spill changes in shader-db are from Dolphin uber shaders, despite some small regressions the change is clearly positive. V2: insert new constant after any phis in the use->parent_instr->type == nir_instr_type_phi path. v3: - use nir_after_block_before_jump() for inserting const - check dominance of phi uses correctly v4: - create some helpers as suggested by Jason. v5 (Jason Ekstrand): - Use LIST_ENTRY to get the phi src shader-db results IVB: total instructions in shared programs: 9999201 -> 9993483 (-0.06%) instructions in affected programs: 163235 -> 157517 (-3.50%) helped: 132 HURT: 2 total cycles in shared programs: 231670754 -> 219476091 (-5.26%) cycles in affected programs: 143424120 -> 131229457 (-8.50%) helped: 115 HURT: 24 total spills in shared programs: 4383 -> 4370 (-0.30%) spills in affected programs: 1656 -> 1643 (-0.79%) helped: 9 HURT: 18 total fills in shared programs: 4610 -> 4581 (-0.63%) fills in affected programs: 374 -> 345 (-7.75%) helped: 6 HURT: 0 Reviewed-by: Jason Ekstrand --- diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index bf4bd916d27..599f469a714 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -2353,6 +2353,36 @@ nir_after_block_before_jump(nir_block *block) } } +static inline nir_cursor +nir_before_src(nir_src *src, bool is_if_condition) +{ + if (is_if_condition) { + nir_block *prev_block = + nir_cf_node_as_block(nir_cf_node_prev(&src->parent_if->cf_node)); + assert(!nir_block_ends_in_jump(prev_block)); + return nir_after_block(prev_block); + } else if (src->parent_instr->type == nir_instr_type_phi) { +#ifndef NDEBUG + nir_phi_instr *cond_phi = nir_instr_as_phi(src->parent_instr); + bool found = false; + nir_foreach_phi_src(phi_src, cond_phi) { + if (phi_src->src.ssa == src->ssa) { + found = true; + break; + } + } + assert(found); +#endif + /* The LIST_ENTRY macro is a generic container-of macro, it just happens + * to have a more specific name. + */ + nir_phi_src *phi_src = LIST_ENTRY(nir_phi_src, src, src); + return nir_after_block_before_jump(phi_src->pred); + } else { + return nir_before_instr(src->parent_instr); + } +} + static inline nir_cursor nir_before_cf_node(nir_cf_node *node) { diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index dacf2d6c667..512fd92575b 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -369,6 +369,73 @@ opt_if_loop_terminator(nir_if *nif) return true; } +static void +replace_if_condition_use_with_const(nir_builder *b, nir_src *use, + nir_const_value nir_boolean, + bool if_condition) +{ + /* Create const */ + nir_ssa_def *const_def = nir_build_imm(b, 1, 32, nir_boolean); + + /* Rewrite use to use const */ + nir_src new_src = nir_src_for_ssa(const_def); + if (if_condition) + nir_if_rewrite_condition(use->parent_if, new_src); + else + nir_instr_rewrite_src(use->parent_instr, use, new_src); +} + +static bool +evaluate_if_condition(nir_if *nif, nir_cursor cursor, uint32_t *value) +{ + nir_block *use_block = nir_cursor_current_block(cursor); + if (nir_block_dominates(nir_if_first_then_block(nif), use_block)) { + *value = NIR_TRUE; + return true; + } else if (nir_block_dominates(nir_if_first_else_block(nif), use_block)) { + *value = NIR_FALSE; + return true; + } else { + return false; + } +} + +static bool +evaluate_condition_use(nir_builder *b, nir_if *nif, nir_src *use_src, + bool is_if_condition) +{ + bool progress = false; + + nir_const_value value; + b->cursor = nir_before_src(use_src, is_if_condition); + + if (evaluate_if_condition(nif, b->cursor, &value.u32[0])) { + replace_if_condition_use_with_const(b, use_src, value, is_if_condition); + progress = true; + } + + return progress; +} + +static bool +opt_if_evaluate_condition_use(nir_builder *b, nir_if *nif) +{ + bool progress = false; + + /* Evaluate any uses of the if condition inside the if branches */ + assert(nif->condition.is_ssa); + nir_foreach_use_safe(use_src, nif->condition.ssa) { + progress |= evaluate_condition_use(b, nif, use_src, false); + } + + nir_foreach_if_use_safe(use_src, nif->condition.ssa) { + if (use_src->parent_if != nif) + progress |= evaluate_condition_use(b, nif, use_src, true); + } + + return progress; +} + static bool opt_if_cf_list(nir_builder *b, struct exec_list *cf_list) { @@ -402,6 +469,41 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list) return progress; } +/** + * These optimisations depend on nir_metadata_block_index and therefore must + * not do anything to cause the metadata to become invalid. + */ +static bool +opt_if_safe_cf_list(nir_builder *b, 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_if_safe_cf_list(b, &nif->then_list); + progress |= opt_if_safe_cf_list(b, &nif->else_list); + progress |= opt_if_evaluate_condition_use(b, nif); + break; + } + + case nir_cf_node_loop: { + nir_loop *loop = nir_cf_node_as_loop(cf_node); + progress |= opt_if_safe_cf_list(b, &loop->body); + break; + } + + case nir_cf_node_function: + unreachable("Invalid cf type"); + } + } + + return progress; +} + bool nir_opt_if(nir_shader *shader) { @@ -414,6 +516,12 @@ nir_opt_if(nir_shader *shader) nir_builder b; nir_builder_init(&b, function->impl); + nir_metadata_require(function->impl, nir_metadata_block_index | + nir_metadata_dominance); + progress = opt_if_safe_cf_list(&b, &function->impl->body); + 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);