<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Wed, Aug 29, 2018 at 6:48 PM Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com">tarceri@itsqueeze.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">v2: - only allow nir_op_inot or nir_op_b2i when alu input is 1. - use some helpers as suggested by Jason. v3: - evaluate alu op for single input alu ops - add helper function to decide if to propagate through alu - make use of nir_before_src in another spot shader-db IVB results: total instructions in shared programs: 9993483 -> 9993472 (-0.00%) instructions in affected programs: 1300 -> 1289 (-0.85%) helped: 11 HURT: 0 total cycles in shared programs: 219476091 -> 219476059 (-0.00%) cycles in affected programs: 7675 -> 7643 (-0.42%) helped: 10 HURT: 1 --- src/compiler/nir/nir_opt_if.c | 145 ++++++++++++++++++++++++++++++++-- 1 file changed, 139 insertions(+), 6 deletions(-) diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index 11c6693d302..9e9d8edda21 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -23,6 +23,7 @@ #include "nir.h" #include "nir/nir_builder.h" +#include "nir_constant_expressions.h" #include "nir_control_flow.h" #include "nir_loop_analyze.h" @@ -403,9 +404,127 @@ evaluate_if_condition(nir_if *nif, nir_cursor cursor, uint32_t *value) } } +/* + * This propagates if condition evaluation down the chain of some alu + * instructions. For example by checking the use of some of the following alu + * instruction we can eventually replace ssa_107 with NIR_TRUE. + * + * loop { + * block block_1: + * vec1 32 ssa_85 = load_const (0x00000002) + * vec1 32 ssa_86 = ieq ssa_48, ssa_85 + * vec1 32 ssa_87 = load_const (0x00000001) + * vec1 32 ssa_88 = ieq ssa_48, ssa_87 + * vec1 32 ssa_89 = ior ssa_86, ssa_88 + * vec1 32 ssa_90 = ieq ssa_48, ssa_0 + * vec1 32 ssa_91 = ior ssa_89, ssa_90 + * if ssa_86 { + * block block_2: + * ... + * break + * } else { + * block block_3: + * } + * block block_4: + * if ssa_88 { + * block block_5: + * ... + * break + * } else { + * block block_6: + * } + * block block_7: + * if ssa_90 { + * block block_8: + * ... + * break + * } else { + * block block_9: + * } + * block block_10: + * vec1 32 ssa_107 = inot ssa_91 + * if ssa_107 { + * block block_11: + * break + * } else { + * block block_12: + * } + * } + */ static bool -evaluate_condition_use(nir_if *nif, nir_src *use_src, void *mem_ctx, - bool is_if_condition) +propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src, + nir_src *alu_use, nir_alu_instr *alu, void *mem_ctx, + bool is_if_condition) +{ + bool progress = false; + + uint32_t bool_value; + b->cursor = nir_before_src(alu_use, is_if_condition); + if (nir_op_infos[alu->op].num_inputs == 1) { + assert(alu->op == nir_op_inot || alu->op == nir_op_b2i); + + if (evaluate_if_condition(nif, b->cursor, &bool_value)) { + nir_const_value bool_src; + bool_src.u32[0] = bool_value; </blockquote><div> </div><div>Since you're just going to put bool_value in bool_src and const_value below, why not just have bool_value be a nir_const_value and do evaluate_if_condition(nif, b->cursor, &bool_value.u32[0]) and be done with it? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + unsigned bit_size = nir_src_bit_size(alu->src[0].src); </blockquote><div> </div><div>This had better be 32 or we're toast because we're assuming a uint32_t the whole time. Maybe make this an assert instead? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + nir_const_value result = + nir_eval_const_opcode(alu->op, 1, bit_size, &bool_src); + + replace_if_condition_use_with_const(alu_use, mem_ctx, b->cursor, + result.u32[0], is_if_condition); + progress = true; + } + } else { + assert(alu->op == nir_op_ior || alu->op == nir_op_iand); + + if (evaluate_if_condition(nif, b->cursor, &bool_value)) { + nir_ssa_def *def[2]; + for (unsigned i = 0; i < 2; i++) { + if (alu->src[i].src.ssa == use_src->ssa) { + nir_const_value const_value; + const_value.u32[0] = bool_value; + + def[i] = nir_build_imm(b, 1, 32, const_value); </blockquote><div> </div><div>We assume 32 here, for instance.</div><div> </div><div>With the above two clean-ups,</div><div> </div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + } else { + def[i] = alu->src[i].src.ssa; + } + } + + nir_ssa_def *nalu = + nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL); + + /* Rewrite use to use new alu instruction */ + nir_src new_src = nir_src_for_ssa(nalu); + + if (is_if_condition) + nir_if_rewrite_condition(alu_use->parent_if, new_src); + else + nir_instr_rewrite_src(alu_use->parent_instr, alu_use, new_src); + + progress = true; + } + } + + return progress; +} + +static bool +can_propagate_through_alu(nir_src *src) +{ + if (src->parent_instr->type == nir_instr_type_alu && + (nir_instr_as_alu(src->parent_instr)->op == nir_op_ior || + nir_instr_as_alu(src->parent_instr)->op == nir_op_iand || + nir_instr_as_alu(src->parent_instr)->op == nir_op_inot || + nir_instr_as_alu(src->parent_instr)->op == nir_op_b2i)) + return true; + + return false; +} + +static bool +evaluate_condition_use(nir_builder *b, nir_if *nif, nir_src *use_src, + void *mem_ctx, bool is_if_condition) { bool progress = false; @@ -417,23 +536,37 @@ evaluate_condition_use(nir_if *nif, nir_src *use_src, void *mem_ctx, progress = true; } + if (!is_if_condition && can_propagate_through_alu(use_src)) { + nir_alu_instr *alu = nir_instr_as_alu(use_src->parent_instr); + + nir_foreach_use_safe(alu_use, &alu->dest.dest.ssa) { + progress |= propagate_condition_eval(b, nif, use_src, alu_use, alu, + mem_ctx, false); + } + + nir_foreach_if_use_safe(alu_use, &alu->dest.dest.ssa) { + progress |= propagate_condition_eval(b, nif, use_src, alu_use, alu, + mem_ctx, true); + } + } + return progress; } static bool -opt_if_evaluate_condition_use(nir_if *nif, void *mem_ctx) +opt_if_evaluate_condition_use(nir_builder *b, 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); + progress |= evaluate_condition_use(b, 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); + progress |= evaluate_condition_use(b, nif, use_src, mem_ctx, true); } return progress; @@ -489,7 +622,7 @@ opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list, void *mem_ctx) 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); + progress |= opt_if_evaluate_condition_use(b, nif, mem_ctx); break; } -- 2.17.1 </blockquote></div></div>