[Mesa-dev] [PATCH] nir: allow propagation of if evaluation for bcsel

[Jason Ekstrand]

On Thu, Nov 1, 2018 at 4:53 PM Timothy Arceri <tarceri at itsqueeze.com> wrote:

> Cc: Jason Ekstrand <jason at jlekstrand.net>
> ---
>  src/compiler/nir/nir_opt_if.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
> index 1fe95e53766..50e8947eaa1 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);
> @@ -472,14 +472,22 @@ propagate_condition_eval(nir_builder *b, nir_if
> *nif, nir_src *use_src,
>  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;
> +   if (src->parent_instr->type != nir_instr_type_alu)
> +      return false;
>

We can do "nir_alu_instr *alu = nir_instr_as_alu(src->parent_instr)" here
and simplify the code below.


> -   return false;
> +   switch (nir_instr_as_alu(src->parent_instr)->op) {
> +      case nir_op_ior:
> +      case nir_op_iand:
> +      case nir_op_inot:
> +      case nir_op_b2i:
> +         return true;
> +      case nir_op_bcsel:
> +         if (src == &nir_instr_as_alu(src->parent_instr)->src[0].src)
> +            return true;
> +         /* fall through */
>

There's no good reason for the fall-through.  Just return false here.  Or,
better yet, "return src == &alu->src[0].src"

With those changes,

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


> +      default:
> +         return false;
> +   }
>  }
>
>  static bool
> --
> 2.19.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181101/35dcad72/attachment-0001.html>