[Mesa-dev] [PATCH 07/31] nir/opt_if: Rework condition propagation
Iago Toral
itoral at igalia.com
Wed Oct 31 08:13:45 UTC 2018
On Wed, 2018-10-31 at 13:22 +1100, Timothy Arceri wrote:
> On 31/10/18 1:23 am, Jason Ekstrand wrote:
> > Weird. I didn't expect this patch to have any impact whatsoever.
> > I
> > thought it was just moving around the way we emit stuff.
>
> I think I've spotted the problem. Iago does patch 1 help with the
> regressions you are seeing.
>
> https://patchwork.freedesktop.org/series/51789/
Yes, patch 1 fixes the problem. Thanks Timothy!
Iago
> >
> > On October 30, 2018 08:40:01 Iago Toral <itoral at igalia.com> wrote:
> >
> > > Jason, JFYI, I have been looking into the cases where the boolean
> > > bitsize lowering pass was producing worse instruction counts that
> > > the
> > > default 32-bit pass and I have tracked it down to this patch.
> > > Reverting
> > > this makes the instruction count much better for some tests, I'll
> > > check
> > > why this happens tomorrow.
> > >
> > > Iago
> > >
> > > On Mon, 2018-10-22 at 17:13 -0500, Jason Ekstrand wrote:
> > > > Instead of doing our own constant folding, we just emit
> > > > instructions
> > > > and
> > > > let constant folding happen. This is substantially simpler and
> > > > lets
> > > > us
> > > > use the nir_imm_bool helper instead of dealing with the
> > > > const_value's
> > > > ourselves.
> > > > ---
> > > > src/compiler/nir/nir_opt_if.c | 91 ++++++++++++---------------
> > > > ------
> > > > --
> > > > 1 file changed, 30 insertions(+), 61 deletions(-)
> > > >
> > > > diff --git a/src/compiler/nir/nir_opt_if.c
> > > > b/src/compiler/nir/nir_opt_if.c
> > > > index 0c94aa170b5..60368a0259e 100644
> > > > --- a/src/compiler/nir/nir_opt_if.c
> > > > +++ b/src/compiler/nir/nir_opt_if.c
> > > > @@ -377,31 +377,15 @@ 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)
> > > > +evaluate_if_condition(nir_if *nif, nir_cursor cursor, bool
> > > > *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;
> > > > + *value = true;
> > > > return true;
> > > > } else if
> > > > (nir_block_dominates(nir_if_first_else_block(nif),
> > > > use_block)) {
> > > > - *value = NIR_FALSE;
> > > > + *value = false;
> > > > return true;
> > > > } else {
> > > > return false;
> > > > @@ -460,52 +444,31 @@ propagate_condition_eval(nir_builder *b,
> > > > nir_if
> > > > *nif, nir_src *use_src,
> > > > nir_src *alu_use, nir_alu_instr *alu,
> > > > bool is_if_condition)
> > > > {
> > > > - bool progress = false;
> > > > + bool bool_value;
> > > > + if (!evaluate_if_condition(nif, b->cursor, &bool_value))
> > > > + return false;
> > > >
> > > > - nir_const_value 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.u32[0]))
> > > > {
> > > > - assert(nir_src_bit_size(alu->src[0].src) == 32);
> > > > -
> > > > - nir_const_value result =
> > > > - nir_eval_const_opcode(alu->op, 1, 32,
> > > > &bool_value);
> > > >
> > > > - replace_if_condition_use_with_const(b, alu_use,
> > > > result,
> > > > - is_if_condition);
> > > > - progress = true;
> > > > + nir_ssa_def *def[2] = { };
> > > > + for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs;
> > > > i++) {
> > > > + if (alu->src[i].src.ssa == use_src->ssa) {
> > > > + def[i] = nir_imm_bool(b, bool_value);
> > > > + } else {
> > > > + def[i] = alu->src[i].src.ssa;
> > > > }
> > > > - } else {
> > > > - assert(alu->op == nir_op_ior || alu->op == nir_op_iand);
> > > > -
> > > > - if (evaluate_if_condition(nif, b->cursor,
> > > > &bool_value.u32[0]))
> > > > {
> > > > - nir_ssa_def *def[2];
> > > > - for (unsigned i = 0; i < 2; i++) {
> > > > - if (alu->src[i].src.ssa == use_src->ssa) {
> > > > - def[i] = nir_build_imm(b, 1, 32, bool_value);
> > > > - } 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);
> > > > + }
> > > > + nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0],
> > > > def[1],
> > > > NULL, NULL);
> > > >
> > > > - 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);
> > > > + /* Rewrite use to use new alu instruction */
> > > > + nir_src new_src = nir_src_for_ssa(nalu);
> > > >
> > > > - progress = true;
> > > > - }
> > > > - }
> > > > + 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);
> > > >
> > > > - return progress;
> > > > + return true;
> > > > }
> > > >
> > > > static bool
> > > > @@ -527,11 +490,17 @@ evaluate_condition_use(nir_builder *b,
> > > > nir_if
> > > > *nif, nir_src *use_src,
> > > > {
> > > > 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);
> > > > + bool bool_value;
> > > > + if (evaluate_if_condition(nif, b->cursor, &bool_value)) {
> > > > + /* Rewrite use to use const */
> > > > + nir_src imm_src = nir_src_for_ssa(nir_imm_bool(b,
> > > > bool_value));
> > > > + if (is_if_condition)
> > > > + nir_if_rewrite_condition(use_src->parent_if,
> > > > imm_src);
> > > > + else
> > > > + nir_instr_rewrite_src(use_src->parent_instr, use_src,
> > > > imm_src);
> > > > +
> > > > progress = true;
> > > > }
> > > >
> >
> >
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
More information about the mesa-dev
mailing list