Agreed, I didn't see that check_explicit_uniform_locations() was only used in link_shaders(). I will submit a v2 with those changes.
On Thu, Apr 7, 2016 at 11:24 PM, Timothy Arceri < timothy.arc...@collabora.com> wrote: > 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 <cheme...@gmail.com> > > > > --- > > 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; > > 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 > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
