[Mesa-dev] [PATCH] glsl: move uniform calculation to link_uniforms

[Matt Turner]

On Fri, Jan 15, 2016 at 9:40 PM, Tapani Pälli <tapani.palli at intel.com> wrote:
> Patch moves uniform calculation to happen during link_uniforms, this
> is possible with help of UniformRemapTable that has all the reserved
> locations.
>
> Location assignment for implicit locations is changed so that we
> utilize also the 'holes' that explicit uniform location assignment
> might have left in UniformRemapTable, this makes it possible to fit
> more uniforms as previously we were lazy here and wasting space.
>
> Fixes following CTS tests:
>    ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max
>    ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max-array
>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>  src/glsl/link_uniforms.cpp | 78 ++++++++++++++++++++++++++++++++++++++++------
>  src/glsl/linker.cpp        | 19 ++++-------
>  src/glsl/linker.h          |  3 +-
>  3 files changed, 77 insertions(+), 23 deletions(-)
>
> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> index 33b2d4c..91e973a 100644
> --- a/src/glsl/link_uniforms.cpp
> +++ b/src/glsl/link_uniforms.cpp
> @@ -1057,9 +1057,31 @@ assign_hidden_uniform_slot_id(const char *name, unsigned hidden_id,
>     uniform_size->map->put(hidden_uniform_start + hidden_id, name);
>  }
>
> +/**
> + * Search UniformRemapTable for empty block big enough to hold given uniform.
> + * TODO Optimize this algorithm later if it turns out to be a major bottleneck.
> + */
> +static int
> +seek_empty_block(struct gl_shader_program *prog,

I might name this "find" instead of "seek"

> +                 struct gl_uniform_storage *uniform)
> +{
> +   const unsigned entries = MAX2(1, uniform->array_elements);
> +   for (unsigned i = 0, j; i < prog->NumUniformRemapTable; i++)

This function *needs* braces! Four nesting levels and only two of them
include braces.

Shouldn't j be declared in the inner for-loop? I don't think j's
larger scope is needed by this code.

> +      if (prog->UniformRemapTable[i] == NULL) {
> +         for (j = i; j < entries && j < prog->NumUniformRemapTable; j++)
> +            if (prog->UniformRemapTable[j] != NULL) {
> +               i = j;
> +               break;
> +            }
> +         return i;
> +      }
> +   return -1;
> +}