[Mesa-dev] [PATCH] compiler/glsl: Fix uniform location counting.

[Tapani Pälli]

On 02/17/2016 04:27 PM, Manolova, Plamena wrote:
> Thank you for reviewing. The change has been run through both Piglit 
> and CTS. I'll ask Tapani to take another look just in case.
>

LGTM and passes all the tests, nice work!

> On Wed, Feb 17, 2016 at 4:20 PM, Ilia Mirkin <imirkin at alum.mit.edu 
> <mailto:imirkin at alum.mit.edu>> wrote:
>
>     On Wed, Feb 17, 2016 at 8:49 AM, Plamena Manolova
>     <plamena.manolova at intel.com <mailto:plamena.manolova at intel.com>>
>     wrote:
>     > This patch moves the calculation of current uniforms to
>     > link_uniforms, which makes use of UniformRemapTable which
>     > stores all the reserved uniform locations.
>     >
>     > Location assignment for implicit uniforms now tries to use
>     > any gaps left in the table after the location assignment
>     > for explicit uniforms. This gives us more space to store more
>     > uniforms.
>     >
>     > Patch is based on earlier patch with following changes/additions:
>     >
>     >    1: Move the counting of explicit locations to
>     >       check_explicit_uniform_locations and then pass
>     >       the number to link_assign_uniform_locations.
>     >    2: Count the number of empty slots in UniformRemapTable
>     >       and store them in a list_head.
>     >    3: Try to find an empty slot for implicit locations from
>     >       the list, if that fails resize UniformRemapTable.
>     >
>     > 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
>     <mailto:tapani.palli at intel.com>>
>     > Signed-off-by: Plamena Manolova <plamena.manolova at intel.com
>     <mailto:plamena.manolova at intel.com>>
>     > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93696
>     > ---
>     >  src/compiler/glsl/link_uniforms.cpp | 80
>     ++++++++++++++++++++++++++++++++-----
>     >  src/compiler/glsl/linker.cpp        | 70
>     +++++++++++++++++++++-----------
>     >  src/compiler/glsl/linker.h          | 17 +++++++-
>     >  src/mesa/main/mtypes.h              |  8 ++++
>     >  4 files changed, 140 insertions(+), 35 deletions(-)
>     >
>     > diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
>     > index bad1c17..5326bfd 100644
>     > --- a/src/compiler/glsl/linker.cpp
>     > +++ b/src/compiler/glsl/linker.cpp
>     > @@ -4129,6 +4148,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>     >
>     >     tfeedback_decl *tfeedback_decls = NULL;
>     >     unsigned num_tfeedback_decls =
>     prog->TransformFeedback.NumVarying;
>     > +   unsigned int num_explicit_uniform_locs = 0;
>
>     This initializer seems unnecessary, since you initialize it
>     unconditionally further down. Otherwise this change is
>
>     Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu
>     <mailto:imirkin at alum.mit.edu>>
>
>     I assume you've also run this through piglit (if not, please do). It
>     may be good for Tapani to have another look at this since he
>     originally tried to implement this and is likely more aware of the
>     various pitfalls.
>
>     Cheers,
>
>       -ilia
>
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160218/ab3ee63d/attachment.html>