[Mesa-dev] [PATCH] mesa: faster validation of sampler unit mapping for SSO

[Timothy Arceri]

On Sat, 2016-06-25 at 10:32 +0200, Gregory Hainaut wrote:
> Hello Timothy,
> 
> On my use case: high FS frequency switches. Few sampler.
> Perf event (relative to nouveau_dri.so) goes from 5.01% to 1.68% for
> the _mesa_sampler_uniforms_pipeline_are_valid function.
> 

I added this to the commit log and pushed. Thanks.

> Unfortunately it didn't translate into measurable speed change. I
> have
> too much FPS variation (cache? temperature, GPU turbo?).
> 
> Honestly I was reluctant to send the patch, but the code isn't more
> complex. I let's you judge.
> 
> Gregory
> 
> On 6/25/16, Timothy Arceri <timothy.arceri at collabora.com> wrote:
> > On Fri, 2016-06-24 at 10:07 +0200, Gregory Hainaut wrote:
> > > Code was inspired from _mesa_update_shader_textures_used
> > > 
> > > However unlike _mesa_update_shader_textures_used that only check
> > > for
> > > a single
> > > stage, it will check all stages.
> > > 
> > > It avoids to loop on all uniforms, only active samplers are
> > > checked.
> > 
> > Hi Gregory,
> > 
> > Is there any measurable change as a result of this? Normally
> > performance changes should come with some results in the commit
> > message.
> > 
> > Tim
> > 
> > > 
> > > Signed-off-by: Gregory Hainaut <gregory.hainaut at gmail.com>
> > > ---
> > >  src/mesa/main/uniform_query.cpp | 69 ++++++++++++++++++---------
> > > ----
> > > ----------
> > >  1 file changed, 31 insertions(+), 38 deletions(-)
> > > 
> > > diff --git a/src/mesa/main/uniform_query.cpp
> > > b/src/mesa/main/uniform_query.cpp
> > > index 127f097..4fa4e6e 100644
> > > --- a/src/mesa/main/uniform_query.cpp
> > > +++ b/src/mesa/main/uniform_query.cpp
> > > @@ -1068,58 +1068,51 @@
> > > _mesa_sampler_uniforms_pipeline_are_valid(struct
> > > gl_pipeline_object
> > > *pipeline)
> > >      *         - The number of active samplers in the program
> > > exceeds
> > > the
> > >      *           maximum number of texture image units allowed."
> > >      */
> > > +
> > > +   GLbitfield mask;
> > > +   GLbitfield TexturesUsed[MAX_COMBINED_TEXTURE_IMAGE_UNITS];
> > > +   struct gl_shader *shader;
> > >     unsigned active_samplers = 0;
> > >     const struct gl_shader_program **shProg =
> > >        (const struct gl_shader_program **) pipeline-
> > > >CurrentProgram;
> > > 
> > > -   const glsl_type
> > > *unit_types[MAX_COMBINED_TEXTURE_IMAGE_UNITS];
> > > -   memset(unit_types, 0, sizeof(unit_types));
> > > +
> > > +   memset(TexturesUsed, 0, sizeof(TexturesUsed));
> > > 
> > >     for (unsigned idx = 0; idx < ARRAY_SIZE(pipeline-
> > > > CurrentProgram); idx++) {
> > >        if (!shProg[idx])
> > >           continue;
> > > 
> > > -      for (unsigned i = 0; i < shProg[idx]->NumUniformStorage;
> > > i++)
> > > {
> > > -         const struct gl_uniform_storage *const storage =
> > > -            &shProg[idx]->UniformStorage[i];
> > > +      shader = shProg[idx]->_LinkedShaders[idx];
> > > +      if (!shader || !shader->Program)
> > > +         continue;
> > > 
> > > -         if (!storage->type->is_sampler())
> > > +      mask = shader->Program->SamplersUsed;
> > > +      while (mask) {
> > > +         const int s = u_bit_scan(&mask);
> > > +         GLuint unit = shader->SamplerUnits[s];
> > > +         GLuint tgt = shader->SamplerTargets[s];
> > > +
> > > +         /* FIXME: Samplers are initialized to 0 and Mesa
> > > doesn't do
> > > a
> > > +          * great job of eliminating unused uniforms currently
> > > so
> > > for now
> > > +          * don't throw an error if two sampler types both point
> > > to
> > > 0.
> > > +          */
> > > +         if (unit == 0)
> > >              continue;
> > > 
> > > -         active_samplers++;
> > > -
> > > -         const unsigned count = MAX2(1, storage-
> > > >array_elements);
> > > -         for (unsigned j = 0; j < count; j++) {
> > > -            const unsigned unit = storage->storage[j].i;
> > > -
> > > -            /* FIXME: Samplers are initialized to 0 and Mesa
> > > doesn't
> > > do a
> > > -             * great job of eliminating unused uniforms
> > > currently so
> > > for now
> > > -             * don't throw an error if two sampler types both
> > > point
> > > to 0.
> > > -             */
> > > -            if (unit == 0)
> > > -               continue;
> > > -
> > > -            /* The types of the samplers associated with a
> > > particular texture
> > > -             * unit must be an exact match.  Page 74 (page 89 of
> > > the
> > > PDF) of
> > > -             * the OpenGL 3.3 core spec says:
> > > -             *
> > > -             *     "It is not allowed to have variables of
> > > different
> > > sampler
> > > -             *     types pointing to the same texture image unit
> > > within a
> > > -             *     program object."
> > > -             */
> > > -            if (unit_types[unit] == NULL) {
> > > -               unit_types[unit] = storage->type;
> > > -            } else if (unit_types[unit] != storage->type) {
> > > -               pipeline->InfoLog =
> > > -                  ralloc_asprintf(pipeline,
> > > -                                  "Texture unit %d is accessed
> > > both
> > > as %s "
> > > -                                  "and %s",
> > > -                                  unit, unit_types[unit]->name,
> > > -                                  storage->type->name);
> > > -               return false;
> > > -            }
> > > +         if (TexturesUsed[unit] & ~(1 << tgt)) {
> > > +            pipeline->InfoLog =
> > > +               ralloc_asprintf(pipeline,
> > > +                     "Program %d: "
> > > +                     "Texture unit %d is accessed with 2
> > > different
> > > types",
> > > +                     shProg[idx]->Name, unit);
> > > +            return false;
> > >           }
> > > +
> > > +         TexturesUsed[unit] |= (1 << tgt);
> > >        }
> > > +
> > > +      active_samplers += shader->num_samplers;
> > >     }
> > > 
> > >     if (active_samplers > MAX_COMBINED_TEXTURE_IMAGE_UNITS) {
> >