[Mesa-dev] [PATCH 2/2] nir: allow propagation of if evaluation to all alu instructions

[Timothy Arceri]

On 1/11/18 1:28 am, Jason Ekstrand wrote:
> On Tue, Oct 30, 2018 at 9:17 PM Timothy Arceri <tarceri at itsqueeze.com 
> <mailto:tarceri at itsqueeze.com>> wrote:
> 
>     With the simplifications to this pass in a3b4cb34589e2f1a68 we
>     can allow any alu instruction to be processed. For one this can
>     potentially help with bcsels.
> 
> 
> Do we want to?  I believe that this patch is correct and I agree that we 
> now can but then we get into some heuristics as to when it is or isn't 
> useful.  With iand, ior, and inot it's obviously useful because, for 
> boolean values, those instructions can be sort of half-folded, i.e. we 
> know that opt_algebraic will look at them and replace them with either 
> the other source or a constant.  With bcsel that's true if the condition 
> is used in src[0] but not for src[1] or src[2].  For other instructions, 
> it's even less obvious that it helps.
> 
> I'm not saying I'm opposed.  Just thinking out loud.

I'm not sure either. Ian (Ccing) was asking about support for bcsel when 
I first wrote the series, I held off sending this patch in the past (due 
to no shader-db change) but since I was looking at the code again I 
thought I might as well send it out and get comments.

I'm happy to hold it back for now but I think we still have many 
opportunities for improving bcsels so maybe it will be useful in future.

> 
> --Jason
> 
>     No shader-db change.
>     ---
>       src/compiler/nir/nir_opt_if.c | 20 ++++----------------
>       1 file changed, 4 insertions(+), 16 deletions(-)
> 
>     diff --git a/src/compiler/nir/nir_opt_if.c
>     b/src/compiler/nir/nir_opt_if.c
>     index 1fe95e53766..38489676e6b 100644
>     --- a/src/compiler/nir/nir_opt_if.c
>     +++ b/src/compiler/nir/nir_opt_if.c
>     @@ -448,7 +448,7 @@ propagate_condition_eval(nir_builder *b, nir_if
>     *nif, nir_src *use_src,
>          if (!evaluate_if_condition(nif, b->cursor, &bool_value))
>             return false;
> 
>     -   nir_ssa_def *def[2] = {0};
>     +   nir_ssa_def *def[4] = {0};
>          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);
>     @@ -456,7 +456,7 @@ propagate_condition_eval(nir_builder *b, nir_if
>     *nif, nir_src *use_src,
>                def[i] = alu->src[i].src.ssa;
>             }
>          }
>     -   nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1],
>     NULL, NULL);
>     +   nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1],
>     def[2], def[3]);
> 
>          /* Rewrite use to use new alu instruction */
>          nir_src new_src = nir_src_for_ssa(nalu);
>     @@ -469,19 +469,6 @@ propagate_condition_eval(nir_builder *b, nir_if
>     *nif, nir_src *use_src,
>          return true;
>       }
> 
>     -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,
>                              bool is_if_condition)
>     @@ -502,7 +489,8 @@ evaluate_condition_use(nir_builder *b, nir_if
>     *nif, nir_src *use_src,
>             progress = true;
>          }
> 
>     -   if (!is_if_condition && can_propagate_through_alu(use_src)) {
>     +   if (!is_if_condition &&
>     +       use_src->parent_instr->type == nir_instr_type_alu) {
>             nir_alu_instr *alu = nir_instr_as_alu(use_src->parent_instr);
> 
>             nir_foreach_use_safe(alu_use, &alu->dest.dest.ssa) {
>     -- 
>     2.17.2
>