[Mesa-dev] [PATCH v2 1/2] nir/lower_io: Return has_indirect flag from get_io_offset().

[Jason Ekstrand]

On Thu, Oct 1, 2015 at 11:13 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> get_io_offset() already walks the dereference chain and discovers
> whether or not we have an indirect; we can just return that rather than
> computing it a second time.  This means moving the call a bit earlier.
>
> More importantly, I'm about to introduce special handling for the
> outermost array index, which would require changing the dereference
> we pass to deref_has_indirect().  Or we can just eliminate that.

I think I actually like what we did in nir_lower_samplers a bit
better.  There we just made indirect a nir_ssa_def* and NULL means no
indirect.  Since these passes all happen while we're in SSA form, that
works pretty well.  Thoughts?

> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/glsl/nir/nir_lower_io.c | 41 ++++++++++++++---------------------------
>  1 file changed, 14 insertions(+), 27 deletions(-)
>
> This replaces patches 3-5 of my recent 23 patch series:
>
> [PATCH 03/23] nir/lower_io: Switch on shader stage for inputs in load_op().
> [PATCH 04/23] nir/lower_io: Make get_io_offset take a nir_deref, not a nir_deref_var.
> [PATCH 05/23] nir: Introduce new nir_intrinsic_load_per_vertex_input intrinsics.
>
> diff --git a/src/glsl/nir/nir_lower_io.c b/src/glsl/nir/nir_lower_io.c
> index f32c09d..80967fc 100644
> --- a/src/glsl/nir/nir_lower_io.c
> +++ b/src/glsl/nir/nir_lower_io.c
> @@ -63,22 +63,9 @@ nir_assign_var_locations(struct exec_list *var_list, unsigned *size,
>     *size = location;
>  }
>
> -static bool
> -deref_has_indirect(nir_deref_var *deref)
> -{
> -   for (nir_deref *tail = deref->deref.child; tail; tail = tail->child) {
> -      if (tail->deref_type == nir_deref_type_array) {
> -         nir_deref_array *arr = nir_deref_as_array(tail);
> -         if (arr->deref_array_type == nir_deref_array_type_indirect)
> -            return true;
> -      }
> -   }
> -
> -   return false;
> -}
> -
>  static unsigned
> -get_io_offset(nir_deref_var *deref, nir_instr *instr, nir_src *indirect,
> +get_io_offset(nir_deref_var *deref, nir_instr *instr,
> +              nir_src *indirect, bool *has_indirect,
>                struct lower_io_state *state)
>  {
>     bool found_indirect = false;
> @@ -122,6 +109,7 @@ get_io_offset(nir_deref_var *deref, nir_instr *instr, nir_src *indirect,
>        }
>     }
>
> +   *has_indirect = found_indirect;
>     return base_offset;
>  }
>
> @@ -169,17 +157,17 @@ nir_lower_io_block(nir_block *block, void *void_state)
>           if (mode != nir_var_shader_in && mode != nir_var_uniform)
>              continue;
>
> -         bool has_indirect = deref_has_indirect(intrin->variables[0]);
> +         nir_src indirect;
> +         bool has_indirect;
> +
> +         unsigned offset = get_io_offset(intrin->variables[0], &intrin->instr,
> +                                         &indirect, &has_indirect, state);
>
>           nir_intrinsic_instr *load =
>              nir_intrinsic_instr_create(state->mem_ctx,
>                                         load_op(mode, has_indirect));
>           load->num_components = intrin->num_components;
>
> -         nir_src indirect;
> -         unsigned offset = get_io_offset(intrin->variables[0],
> -                                         &intrin->instr, &indirect, state);
> -
>           unsigned location = intrin->variables[0]->var->data.driver_location;
>           if (mode == nir_var_uniform) {
>              load->const_index[0] = location;
> @@ -209,7 +197,12 @@ nir_lower_io_block(nir_block *block, void *void_state)
>           if (intrin->variables[0]->var->data.mode != nir_var_shader_out)
>              continue;
>
> -         bool has_indirect = deref_has_indirect(intrin->variables[0]);
> +         nir_src indirect;
> +         bool has_indirect;
> +
> +         unsigned offset = get_io_offset(intrin->variables[0], &intrin->instr,
> +                                         &indirect, &has_indirect, state);
> +         offset += intrin->variables[0]->var->data.driver_location;
>
>           nir_intrinsic_op store_op;
>           if (has_indirect) {
> @@ -221,12 +214,6 @@ nir_lower_io_block(nir_block *block, void *void_state)
>           nir_intrinsic_instr *store = nir_intrinsic_instr_create(state->mem_ctx,
>                                                                   store_op);
>           store->num_components = intrin->num_components;
> -
> -         nir_src indirect;
> -         unsigned offset = get_io_offset(intrin->variables[0],
> -                                         &intrin->instr, &indirect, state);
> -         offset += intrin->variables[0]->var->data.driver_location;
> -
>           store->const_index[0] = offset;
>
>           nir_src_copy(&store->src[0], &intrin->src[0], store);
> --
> 2.5.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev