[Mesa-dev] [RFC 3/4] nir: support indirect indexing samplers in struct arrays

Timothy Arceri t_arceri at yahoo.com.au
Sun Aug 30 21:31:26 PDT 2015


On Sun, 2015-08-30 at 08:27 -0700, Jason Ekstrand wrote:
> 
> On Aug 30, 2015 3:44 AM, "Timothy Arceri" <t_arceri at yahoo.com.au> wrote:
> >
> > As a bonus we get indirect support for arrays of arrays for free.
> > ---
> >  I've marked this as request for comment as although it works enough for 
> to
> >  pass all the piglit tests there are issues as I'm still trying to figure 
> out
> >  nir.
> >
> >  The issues are around the second call to nir_instr_move_src() this call
> >  expects the use_link list not to be empty just so it can then remove all
> >  entries so I add the list_addtail() call above it to work around this.
> >  While this is enough to pass the piglit tests on a non debug build it 
> fails
> >  validation for debug builds.
> >
> >  I'm clearly missing a step and would be greatful to anyone who can point 
> me
> >  in the right direction.
> nir_instr_move_src expects the source to be already in an instruction.  The > reason its correct in the old code is because were stealing it from the > indirect.  For adding a "new" nir_src, use nir_instr_rewrite_src.

Right thanks Jason, and the validation was failing because the list wasn't
being cleared for the old indirect anymore. I've sent a V2.


> --Jason
> >  The validation error:
> >  nir/nir_validate.c:873: postvalidate_ssa_def: Assertion `entry' failed.
> >
> > 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..dbe9e95 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,49 @@ 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));
> > +
> > +            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;
> > +         calc_sampler_offsets(tail->child, instr, array_elements,
> > +                              indirect, b, location, found_indirect);
> >           break;
> >        }
> >
> > @@ -125,15 +86,75 @@ 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;
> > +
> > +      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.
> > +          */
> > +         nir_src indirect_nir_src = nir_src_for_ssa(indirect.ssa);
> > +         list_addtail(&indirect_nir_src.use_link, &indirect_nir_src.ssa
> ->uses);
> > +
> > +         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,
> > +                            &indirect_nir_src);
> > +
> > +         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