On Thu, Jul 19, 2018 at 10:24 AM, Timothy Arceri <tarc...@itsqueeze.com> wrote: > On 19/07/18 12:02, Connor Abbott wrote: >> >> Why not do the more general thing, and evaluate the condition in every >> block dominated by the then and else blocks? That should handle the >> loop and non-loop cases. > > > Can you explain what the advantage would be in doing that? Is it just likely > to reduce the code required?
Yeah, that's it. Plus it works the same for unstructured control flow (if we ever add that). > > >> >> On Thu, Jul 19, 2018 at 8:06 AM, Timothy Arceri <tarc...@itsqueeze.com> >> wrote: >>> >>> Since we know what side of the branch we ended up on we can just >>> replace the use with a constant. >>> >>> All helped shaders are from Unreal Engine 4 besides one shader from >>> Dirt Showdown. >>> >>> V2: make sure we do evaluation when condition is used in else with >>> a single block (we were checking for blocks < the last else >>> block rather than <=) >>> >>> shader-db results SKL: >>> >>> total instructions in shared programs: 13219725 -> 13219643 (<.01%) >>> instructions in affected programs: 28917 -> 28835 (-0.28%) >>> helped: 45 >>> HURT: 0 >>> >>> total cycles in shared programs: 529335971 -> 529334604 (<.01%) >>> cycles in affected programs: 216209 -> 214842 (-0.63%) >>> helped: 45 >>> HURT: 4 >>> >>> Cc: Ian Romanick <i...@freedesktop.org> >>> >>> fix if condition eval for else with a single block >>> --- >>> src/compiler/nir/nir_opt_if.c | 121 ++++++++++++++++++++++++++++++++++ >>> 1 file changed, 121 insertions(+) >>> >>> diff --git a/src/compiler/nir/nir_opt_if.c >>> b/src/compiler/nir/nir_opt_if.c >>> index a52de120ad6..4ed919887ce 100644 >>> --- a/src/compiler/nir/nir_opt_if.c >>> +++ b/src/compiler/nir/nir_opt_if.c >>> @@ -348,6 +348,86 @@ opt_if_loop_terminator(nir_if *nif) >>> return true; >>> } >>> >>> +static void >>> +replace_if_condition_use_with_const(nir_src *use, unsigned nir_boolean, >>> + void *mem_ctx, bool if_condition) >>> +{ >>> + /* Create const */ >>> + nir_load_const_instr *load = nir_load_const_instr_create(mem_ctx, 1, >>> 32); >>> + load->value.u32[0] = nir_boolean; >>> + >>> + if (if_condition) { >>> + nir_instr_insert_before_cf(&use->parent_if->cf_node, >>> &load->instr); >>> + } else if (use->parent_instr->type == nir_instr_type_phi) { >>> + nir_phi_instr *cond_phi = nir_instr_as_phi(use->parent_instr); >>> + >>> + bool UNUSED found = false; >>> + nir_foreach_phi_src(phi_src, cond_phi) { >>> + if (phi_src->src.ssa == use->ssa) { >>> + nir_instr_insert_before_block(phi_src->pred, &load->instr); >>> + found = true; >>> + break; >>> + } >>> + } >>> + assert(found); >>> + } else { >>> + nir_instr_insert_before(use->parent_instr, &load->instr); >>> + } >>> + >>> + /* Rewrite use to use const */ >>> + nir_src new_src = nir_src_for_ssa(&load->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_condition_use(nir_if *nif, nir_src *use_src, void *mem_ctx, >>> + bool if_condition) >>> +{ >>> + bool progress = false; >>> + >>> + nir_block *first_then = nir_if_first_then_block(nif); >>> + if (use_src->parent_instr->block->index > first_then->index) { >>> + nir_block *first_else = nir_if_first_else_block(nif); >>> + if (use_src->parent_instr->block->index < first_else->index) { >>> + replace_if_condition_use_with_const(use_src, NIR_TRUE, mem_ctx, >>> + if_condition); >>> + >>> + progress = true; >>> + } else if (use_src->parent_instr->block->index <= >>> + nir_if_last_else_block(nif)->index) { >>> + replace_if_condition_use_with_const(use_src, NIR_FALSE, >>> mem_ctx, >>> + if_condition); >>> + >>> + progress = true; >>> + } >>> + } >>> + >>> + return progress; >>> +} >>> + >>> +static bool >>> +opt_if_evaluate_condition_use(nir_if *nif, void *mem_ctx) >>> +{ >>> + 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(nif, use_src, mem_ctx, false); >>> + } >>> + >>> + nir_foreach_if_use_safe(use_src, nif->condition.ssa) { >>> + if (use_src->parent_if != nif) >>> + progress |= evaluate_condition_use(nif, use_src, mem_ctx, >>> true); >>> + } >>> + >>> + return progress; >>> +} >>> + >>> static bool >>> opt_if_cf_list(nir_builder *b, struct exec_list *cf_list) >>> { >>> @@ -381,6 +461,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, void >>> *mem_ctx) >>> +{ >>> + 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, mem_ctx); >>> + progress |= opt_if_safe_cf_list(b, &nif->else_list, mem_ctx); >>> + progress |= opt_if_evaluate_condition_use(nif, mem_ctx); >>> + 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, mem_ctx); >>> + break; >>> + } >>> + >>> + case nir_cf_node_function: >>> + unreachable("Invalid cf type"); >>> + } >>> + } >>> + >>> + return progress; >>> +} >>> + >>> bool >>> nir_opt_if(nir_shader *shader) >>> { >>> @@ -393,6 +508,12 @@ nir_opt_if(nir_shader *shader) >>> nir_builder b; >>> nir_builder_init(&b, function->impl); >>> >>> + void *mem_ctx = ralloc_parent(function->impl); >>> + >>> + nir_metadata_require(function->impl, nir_metadata_block_index); >>> + progress = opt_if_safe_cf_list(&b, &function->impl->body, >>> mem_ctx); >>> + nir_metadata_preserve(function->impl, nir_metadata_block_index); >>> + >>> if (opt_if_cf_list(&b, &function->impl->body)) { >>> nir_metadata_preserve(function->impl, nir_metadata_none); >>> >>> -- >>> 2.17.1 >>> >>> _______________________________________________ >>> mesa-dev mailing list >>> mesa-dev@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev