[Mesa-dev] [PATCH 1/2] nir/loop_analyze: used nir_alu_src to track loop limit

[Jason Ekstrand]

On Wed, Jun 19, 2019 at 3:09 AM Timothy Arceri <tarceri at itsqueeze.com>
wrote:

> This helps reduce the amount of abstraction in this pass and allows
> us to retain more information about the src such as any swizzles.
> Retaining the swizzle information is required for a bugfix in the
> following patch.
>
> Fixes: 6772a17acc8e ("nir: Add a loop analysis pass")
> ---
>  src/compiler/nir/nir_loop_analyze.c | 37 +++++++++++++++--------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/src/compiler/nir/nir_loop_analyze.c
> b/src/compiler/nir/nir_loop_analyze.c
> index e85a404da1b..57d2d94cad2 100644
> --- a/src/compiler/nir/nir_loop_analyze.c
> +++ b/src/compiler/nir/nir_loop_analyze.c
> @@ -543,25 +543,26 @@ guess_loop_limit(loop_info_state *state,
> nir_const_value *limit_val,
>  }
>
>  static bool
> -try_find_limit_of_alu(nir_loop_variable *limit, nir_const_value
> *limit_val,
> -                      nir_loop_terminator *terminator, loop_info_state
> *state)
> +try_find_limit_of_alu(nir_alu_src *limit, nir_const_value *limit_val,
> +                      nir_loop_terminator *terminator)
>  {
> -   if(!is_var_alu(limit))
> +   if(limit->src.ssa->parent_instr->type != nir_instr_type_alu)
>        return false;
>
> -   nir_alu_instr *limit_alu = nir_instr_as_alu(limit->def->parent_instr);
> +   nir_alu_instr *limit_alu =
> nir_instr_as_alu(limit->src.ssa->parent_instr);
>
>     if (limit_alu->op == nir_op_imin ||
>         limit_alu->op == nir_op_fmin) {
> -      limit = get_loop_var(limit_alu->src[0].src.ssa, state);
> +      limit = &limit_alu->src[0];
>
> -      if (!is_var_constant(limit))
> -         limit = get_loop_var(limit_alu->src[1].src.ssa, state);
> +      if (limit->src.ssa->parent_instr->type != nir_instr_type_load_const)
> +         limit = &limit_alu->src[1];
>

This is still horribly broken w.r.t swizzles because we're not tracking the
component as we make this or the jump above for [if]min.


>
> -      if (!is_var_constant(limit))
> +      if (limit->src.ssa->parent_instr->type != nir_instr_type_load_const)
>           return false;
>
> -      *limit_val =
> nir_instr_as_load_const(limit->def->parent_instr)->value[0];
> +      *limit_val =
> +         nir_instr_as_load_const(limit->src.ssa->parent_instr)->value[0];
>
>        terminator->exact_trip_count_unknown = true;
>
> @@ -777,19 +778,19 @@ is_supported_terminator_condition(nir_alu_instr *alu)
>
>  static bool
>  get_induction_and_limit_vars(nir_alu_instr *alu, nir_loop_variable **ind,
> -                             nir_loop_variable **limit,
> +                             nir_alu_src **limit,
>                               loop_info_state *state)
>  {
>     bool limit_rhs = true;
>
>     /* We assume that the limit is the "right" operand */
>     *ind = get_loop_var(alu->src[0].src.ssa, state);
> -   *limit = get_loop_var(alu->src[1].src.ssa, state);
> +   *limit = &alu->src[1];
>
>     if ((*ind)->type != basic_induction) {
>        /* We had it the wrong way, flip things around */
>        *ind = get_loop_var(alu->src[1].src.ssa, state);
> -      *limit = get_loop_var(alu->src[0].src.ssa, state);
> +      *limit = &alu->src[0];
>        limit_rhs = false;
>     }
>
> @@ -799,7 +800,7 @@ get_induction_and_limit_vars(nir_alu_instr *alu,
> nir_loop_variable **ind,
>  static void
>  try_find_trip_count_vars_in_iand(nir_alu_instr **alu,
>                                   nir_loop_variable **ind,
> -                                 nir_loop_variable **limit,
> +                                 nir_alu_src **limit,
>                                   bool *limit_rhs,
>                                   loop_info_state *state)
>  {
> @@ -848,7 +849,7 @@ try_find_trip_count_vars_in_iand(nir_alu_instr **alu,
>
>     /* Try the other iand src if needed */
>     if (*ind == NULL || (*ind && (*ind)->type != basic_induction) ||
> -       !is_var_constant(*limit)) {
> +       (*limit)->src.ssa->parent_instr->type !=
> nir_instr_type_load_const) {
>        src = iand->src[1].src.ssa;
>        if (src->parent_instr->type == nir_instr_type_alu) {
>           nir_alu_instr *tmp_alu = nir_instr_as_alu(src->parent_instr);
> @@ -891,7 +892,7 @@ find_trip_count(loop_info_state *state)
>
>        bool limit_rhs;
>        nir_loop_variable *basic_ind = NULL;
> -      nir_loop_variable *limit;
> +      nir_alu_src *limit;
>        if (alu->op == nir_op_inot || alu->op == nir_op_ieq) {
>           nir_alu_instr *new_alu = alu;
>           try_find_trip_count_vars_in_iand(&new_alu, &basic_ind, &limit,
> @@ -931,13 +932,13 @@ find_trip_count(loop_info_state *state)
>
>        /* Attempt to find a constant limit for the loop */
>        nir_const_value limit_val;
> -      if (is_var_constant(limit)) {
> +      if (limit->src.ssa->parent_instr->type ==
> nir_instr_type_load_const) {
>           limit_val =
> -            nir_instr_as_load_const(limit->def->parent_instr)->value[0];
> +
> nir_instr_as_load_const(limit->src.ssa->parent_instr)->value[0];
>        } else {
>           trip_count_known = false;
>
> -         if (!try_find_limit_of_alu(limit, &limit_val, terminator,
> state)) {
> +         if (!try_find_limit_of_alu(limit, &limit_val, terminator)) {
>              /* Guess loop limit based on array access */
>              if (!guess_loop_limit(state, &limit_val, basic_ind)) {
>                 continue;
> --
> 2.21.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190619/f22ad682/attachment-0001.html>