<div dir="ltr"><div><div>Agreed, I didn't see that check_explicit_uniform_locations() was </div>only used in link_shaders(). I will submit a v2 with those changes. </div></div><div class="gmail_extra"> <div class="gmail_quote">On Thu, Apr 7, 2016 at 11:24 PM, Timothy Arceri <<a href="mailto:timothy.arceri@collabora.com" target="_blank">timothy.arceri@collabora.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Thu, 2016-04-07 at 11:22 -0400, Lars Hamre wrote: > NOTES: > * Reposting due to no response last week > * Someone with access will need to commit this patch after the review > process > > check_explicit_uniform_locations() returns a -1 when the extension > ARB_explicit_uniform_location is not found. > > We were storing the result in num_explicit_uniform_locs as an > unsigned int which caused it to be 4294967295 when a -1 was returned. > > This in turn would cause the following error during linking: > error: count of uniform locations > MAX_UNIFORM_LOCATIONS(4294967295 > > 98304) > > Here are the results from running piglit tests/all with this patch: > changes: 178 > fixes: 176 > regressions: 2 > > The two regressions are for the following tests: > glean@glsl1-matrix column check (1) > glean@glsl1-matrix column check (2) > which regress from FAIL to CRASH. > > I am okay with these regressions because the tests are currently > failing due to the aforementioned linker error. > > Signed-off-by: Lars Hamre <<a href="mailto:chemecse@gmail.com">chemecse@gmail.com</a>> > > --- > src/compiler/glsl/linker.cpp | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/src/compiler/glsl/linker.cpp > b/src/compiler/glsl/linker.cpp > index cd35464..a0a9104 100644 > --- a/src/compiler/glsl/linker.cpp > +++ b/src/compiler/glsl/linker.cpp > @@ -4171,7 +4171,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; > + int num_explicit_uniform_locs = 0; > > void *mem_ctx = ralloc_context(NULL); // temporary linker context > > @@ -4354,6 +4354,8 @@ link_shaders(struct gl_context *ctx, struct > gl_shader_program *prog) > } > > num_explicit_uniform_locs = check_explicit_uniform_locations(ctx, > prog); > + if (num_explicit_uniform_locs < 0) > + num_explicit_uniform_locs = 0; </div></div>Thanks for spotting this. However the just hacks around the problem. Since we never do anything with the -1 I would rather see check_explicit_uniform_locations() changed to return an unsigned 0 rather than -1. > link_assign_subroutine_types(prog); > > if (!prog->LinkStatus) > @@ -4585,7 +4587,7 @@ link_shaders(struct gl_context *ctx, struct > gl_shader_program *prog) > > update_array_sizes(prog); > link_assign_uniform_locations(prog, ctx- > >Const.UniformBooleanTrue, > - num_explicit_uniform_locs, > + (unsigned > int)num_explicit_uniform_locs, > ctx- > >Const.MaxUserAssignableUniformLocations); > link_assign_atomic_counter_resources(ctx, prog); > store_fragdepth_layout(prog); > -- > 2.5.5 > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div>