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

[Gregory Hainaut]

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.

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) {
>