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

[Jason Ekstrand]

On Tue, Oct 30, 2018 at 9:17 PM Timothy Arceri <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.

--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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181031/91e4fd34/attachment.html>