[Mesa-dev] [PATCH 07/31] nir/opt_if: Rework condition propagation

[Iago Toral]

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;
>     }
>