[Mesa-dev] [PATCH V3 5/6] nir: support indirect indexing samplers in struct arrays

Jason Ekstrand jason at jlekstrand.net
Thu Sep 10 18:04:12 PDT 2015


On Tue, Sep 1, 2015 at 7:44 PM, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> As a bonus we get indirect support for arrays of arrays for free.
>
> V3: Use nir_instr_rewrite_src() with empty src rather then clearing
> the use_link list directly for the old indirects as suggested by Jason
>
> V2: Fixed validation error in debug build
> ---
>  src/glsl/nir/nir_lower_samplers.cpp | 169 ++++++++++++++++++++----------------
>  1 file changed, 95 insertions(+), 74 deletions(-)
>
> diff --git a/src/glsl/nir/nir_lower_samplers.cpp b/src/glsl/nir/nir_lower_samplers.cpp
> index 9583b45..9736e0f 100644
> --- a/src/glsl/nir/nir_lower_samplers.cpp
> +++ b/src/glsl/nir/nir_lower_samplers.cpp
> @@ -24,6 +24,7 @@
>   */
>
>  #include "nir.h"
> +#include "nir_builder.h"
>  #include "../program.h"
>  #include "program/hash_table.h"
>  #include "ir_uniform.h"
> @@ -35,89 +36,52 @@ extern "C" {
>  #include "program/program.h"
>  }
>
> -static unsigned
> -get_sampler_index(const struct gl_shader_program *shader_program,
> -                  gl_shader_stage stage, const char *name)
> -{
> -   unsigned location;
> -   if (!shader_program->UniformHash->get(location, name)) {
> -      assert(!"failed to find sampler");
> -      return 0;
> -   }
> -
> -   if (!shader_program->UniformStorage[location].sampler[stage].active) {
> -      assert(!"cannot return a sampler");
> -      return 0;
> -   }
> -
> -   return shader_program->UniformStorage[location].sampler[stage].index;
> -}
> -
> +/* Calculate the sampler index based on array indicies and also
> + * calculate the base uniform location for struct members.
> + */
>  static void
> -lower_sampler(nir_tex_instr *instr, const struct gl_shader_program *shader_program,
> -              gl_shader_stage stage, void *mem_ctx)
> +calc_sampler_offsets(nir_deref *tail, nir_tex_instr *instr,
> +                     unsigned *array_elements, nir_src *indirect,
> +                     nir_builder *b, unsigned *location,
> +                     bool *found_indirect)
>  {
> -   if (instr->sampler == NULL)
> -      return;
> -
> -   /* Get the name and the offset */
> -   instr->sampler_index = 0;
> -   char *name = ralloc_strdup(mem_ctx, instr->sampler->var->name);
> -
> -   for (nir_deref *deref = &instr->sampler->deref;
> -        deref->child; deref = deref->child) {
> -      switch (deref->child->deref_type) {
> +   if (tail->child != NULL) {
> +      switch (tail->child->deref_type) {
>        case nir_deref_type_array: {
> -         nir_deref_array *deref_array = nir_deref_as_array(deref->child);
> +         nir_deref_array *deref_array = nir_deref_as_array(tail->child);
>
>           assert(deref_array->deref_array_type != nir_deref_array_type_wildcard);
>
> -         if (deref_array->deref.child) {
> -            ralloc_asprintf_append(&name, "[%u]",
> -               deref_array->deref_array_type == nir_deref_array_type_direct ?
> -                  deref_array->base_offset : 0);
> -         } else {
> -            assert(deref->child->type->base_type == GLSL_TYPE_SAMPLER);
> -            instr->sampler_index = deref_array->base_offset;
> -         }
> +         calc_sampler_offsets(tail->child, instr, array_elements,
> +                              indirect, b, location, found_indirect);
> +         instr->sampler_index += deref_array->base_offset * *array_elements;
>
> -         /* XXX: We're assuming here that the indirect is the last array
> -          * thing we have.  This should be ok for now as we don't support
> -          * arrays_of_arrays yet.
> -          */
>           if (deref_array->deref_array_type == nir_deref_array_type_indirect) {
> -            /* First, we have to resize the array of texture sources */
> -            nir_tex_src *new_srcs = rzalloc_array(instr, nir_tex_src,
> -                                                  instr->num_srcs + 1);
> -
> -            for (unsigned i = 0; i < instr->num_srcs; i++) {
> -               new_srcs[i].src_type = instr->src[i].src_type;
> -               nir_instr_move_src(&instr->instr, &new_srcs[i].src,
> -                                  &instr->src[i].src);
> +            nir_ssa_def *mul =
> +               nir_imul(b, nir_imm_int(b, *array_elements),
> +                        nir_ssa_for_src(b, deref_array->indirect, 1));
> +
> +            nir_instr_rewrite_src(&instr->instr, &deref_array->indirect,
> +                                  NIR_SRC_INIT);
> +
> +            if (*found_indirect) {
> +               indirect->ssa =
> +                  nir_iadd(b, nir_ssa_for_src(b, *indirect, 1), mul);
> +            } else {
> +               *indirect = nir_src_for_ssa(mul);
> +               *found_indirect = true;
>              }
> -
> -            ralloc_free(instr->src);
> -            instr->src = new_srcs;
> -
> -            /* Now we can go ahead and move the source over to being a
> -             * first-class texture source.
> -             */
> -            instr->src[instr->num_srcs].src_type = nir_tex_src_sampler_offset;
> -            instr->num_srcs++;
> -            nir_instr_move_src(&instr->instr,
> -                               &instr->src[instr->num_srcs - 1].src,
> -                               &deref_array->indirect);
> -
> -            instr->sampler_array_size = glsl_get_length(deref->type);
>           }
> -         break;
> +
> +         *array_elements *= glsl_get_length(tail->type);
> +          break;
>        }
>
>        case nir_deref_type_struct: {
> -         nir_deref_struct *deref_struct = nir_deref_as_struct(deref->child);
> -         const char *field = glsl_get_struct_elem_name(deref->type,
> -                                                       deref_struct->index);
> -         ralloc_asprintf_append(&name, ".%s", field);
> +         nir_deref_struct *deref_struct = nir_deref_as_struct(tail->child);
> +         *location += deref_struct->index;

I finally got around to looking at this patch again and most of it
seems fine except for this line.  I don't think you can just increment
location by deref_struct->index.  I think you need to add up all of
the array sizes of all of the previous members.  (Yeah, that's gross
but I'm not thinking of anything better off-hand).

> +         calc_sampler_offsets(tail->child, instr, array_elements,
> +                              indirect, b, location, found_indirect);
>           break;
>        }
>
> @@ -125,15 +89,72 @@ lower_sampler(nir_tex_instr *instr, const struct gl_shader_program *shader_progr
>           unreachable("Invalid deref type");
>           break;
>        }
> +   } else {
> +      return;
> +   }
> +}
> +
> +static void
> +lower_sampler(nir_tex_instr *instr, const struct gl_shader_program *shader_program,
> +              gl_shader_stage stage, nir_builder *builder)
> +{
> +   if (instr->sampler == NULL)
> +      return;
> +
> +   instr->sampler_index = 0;
> +   unsigned location = instr->sampler->var->data.location;
> +
> +   if (instr->sampler->deref.child) {
> +      unsigned array_elements = 1;
> +      nir_src indirect;
> +      bool found_indirect = false;

I may have made this comment before, but wouldn't it be easier to just
have "indirect" be a nir_ssa_def pointer?  Then you wouldn't need to
also have a boolean (NULL == no indirect) and you could completely
side-step any nir_src copying issues.

--Jason

> +
> +      builder->cursor = nir_before_instr(&instr->instr);
> +      calc_sampler_offsets(&instr->sampler->deref, instr,
> +                           &array_elements, &indirect, builder,
> +                           &location, &found_indirect);
> +
> +      if (found_indirect) {
> +         /* First, we have to resize the array of texture sources */
> +         nir_tex_src *new_srcs = rzalloc_array(instr, nir_tex_src,
> +                                               instr->num_srcs + 1);
> +
> +         for (unsigned i = 0; i < instr->num_srcs; i++) {
> +            new_srcs[i].src_type = instr->src[i].src_type;
> +            nir_instr_move_src(&instr->instr, &new_srcs[i].src,
> +                               &instr->src[i].src);
> +         }
> +
> +         ralloc_free(instr->src);
> +         instr->src = new_srcs;
> +
> +         /* Now we can go ahead and move the source over to being a
> +          * first-class texture source.
> +          */
> +         instr->src[instr->num_srcs].src_type = nir_tex_src_sampler_offset;
> +         instr->num_srcs++;
> +         nir_instr_rewrite_src(&instr->instr,
> +                               &instr->src[instr->num_srcs - 1].src,
> +                               indirect);
> +
> +         instr->sampler_array_size = array_elements;
> +      }
> +   }
> +
> +   if (location > shader_program->NumUniformStorage - 1 ||
> +       !shader_program->UniformStorage[location].sampler[stage].active) {
> +      assert(!"cannot return a sampler");
> +      return;
>     }
>
> -   instr->sampler_index += get_sampler_index(shader_program, stage, name);
> +   instr->sampler_index +=
> +      shader_program->UniformStorage[location].sampler[stage].index;
>
>     instr->sampler = NULL;
>  }
>
>  typedef struct {
> -   void *mem_ctx;
> +   nir_builder builder;
>     const struct gl_shader_program *shader_program;
>     gl_shader_stage stage;
>  } lower_state;
> @@ -147,7 +168,7 @@ lower_block_cb(nir_block *block, void *_state)
>        if (instr->type == nir_instr_type_tex) {
>           nir_tex_instr *tex_instr = nir_instr_as_tex(instr);
>           lower_sampler(tex_instr, state->shader_program, state->stage,
> -                       state->mem_ctx);
> +                       &state->builder);
>        }
>     }
>
> @@ -160,7 +181,7 @@ lower_impl(nir_function_impl *impl, const struct gl_shader_program *shader_progr
>  {
>     lower_state state;
>
> -   state.mem_ctx = ralloc_parent(impl);
> +   nir_builder_init(&state.builder, impl);
>     state.shader_program = shader_program;
>     state.stage = stage;
>
> --
> 2.4.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list