[Mesa-dev] [RFCv2 03/13] nir: allow pre-resolved sampler uniform locations

[Timothy Arceri]

On Mon, 2015-11-09 at 07:43 -0500, Rob Clark wrote:
> On Sun, Nov 8, 2015 at 7:58 PM, Timothy Arceri <t_arceri at yahoo.com.au>
> wrote:
> > On Sun, 2015-11-08 at 15:12 -0500, Rob Clark wrote:
> > > From: Rob Clark <robclark at freedesktop.org>
> > > 
> > > With TGSI, the ir_variable::data.location gets fixed up to be a stage
> > > local location (rather than program global).  In this case we need to
> > > skip the UniformStorage[location] lookup.
> > > ---
> > >  src/glsl/nir/nir_lower_samplers.c | 23 ++++++++++++++++-------
> > >  1 file changed, 16 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/src/glsl/nir/nir_lower_samplers.c
> > > b/src/glsl/nir/nir_lower_samplers.c
> > > index 5df79a6..d99ba4c 100644
> > > --- a/src/glsl/nir/nir_lower_samplers.c
> > > +++ b/src/glsl/nir/nir_lower_samplers.c
> > > @@ -130,14 +130,18 @@ lower_sampler(nir_tex_instr *instr, const struct
> > > gl_shader_program *shader_progr
> > >        instr->sampler_array_size = array_elements;
> > >     }
> > > 
> > > -   if (location > shader_program->NumUniformStorage - 1 ||
> > > -       !shader_program->UniformStorage[location].opaque[stage].active)
> > > {
> > > -      assert(!"cannot return a sampler");
> > > -      return;
> > > -   }
> > > +   if (!shader_program) {
> > > +      instr->sampler_index = location;
> > > +   } else {
> > > +      if (location > shader_program->NumUniformStorage - 1 ||
> > > +          !shader_program
> > > ->UniformStorage[location].opaque[stage].active) {
> > > +         assert(!"cannot return a sampler");
> > > +         return;
> > > +      }
> > > 
> > > -   instr->sampler_index +=
> > > -      shader_program->UniformStorage[location].opaque[stage].index;
> > > +      instr->sampler_index =
> > > +         shader_program->UniformStorage[location].opaque[stage].index;
> > 
> > Hi Rob,
> > 
> > This will break arrays as instr->sampler_index is increamented inside
> >  calc_sampler_offsets()
> 
> oh, whoops, I didn't notice that.. ok, that part is easy enough to fix..
> 
> > calc_sampler_offsets() also modifies the value of location is this what
> > you
> > want? I would assume not as we are counting uniforms not just samplers
> > here.
> 
> hmm, tbh I'm not entirely sure..  offhand, what piglit's should I
> check? 

tests/spec/arb_gpu_shader5/execution/sampler_array_indexing

Contains the tests you probably want to try out.

>  I guess it would be easier to debug if it worked correctly
> with glsl_to_tgsi, but I guess I could try to get the non-indirect
> case working..
> 
> BR,
> -R
> 
> > The other thing to note is that glsl to tgsi doesn't handle indirects on
> > structs or arrays of arrays correctly (Ilia was trying to fix this).
> > 
> > Tim
> > 
> > 
> > 
> > > +   }
> > > 
> > >     instr->sampler = NULL;
> > >  }
> > > @@ -177,6 +181,11 @@ lower_impl(nir_function_impl *impl, const struct
> > > gl_shader_program *shader_progr
> > >     nir_foreach_block(impl, lower_block_cb, &state);
> > >  }
> > > 
> > > +/* Call with a null 'shader_program' if uniform locations are
> > 
> > uniform locations -> sampler indices?
> > 
> > > + * already local to the shader, ie. skipping the
> > > + * shader_program->UniformStorage[location].opaque[stage].index
> > > + * lookup
> > > + */
> > >  void
> > >  nir_lower_samplers(nir_shader *shader,
> > >                     const struct gl_shader_program *shader_program)