On Tue, 2016-05-17 at 20:24 -0700, Ian Romanick wrote:
> On 05/17/2016 04:40 PM, Timothy Arceri wrote:
> >
> > On Tue, 2016-05-17 at 15:11 -0700, Ian Romanick wrote:
> > >
> > > From: Ian Romanick <ian.d.roman...@intel.com>
> > >
> > > Previously an SSO pipeline containing only a tessellation control
> > > shader
> > > and a tessellation evaluation shader would not get locations
> > > assigned
> > > for the TCS inputs. This would lead to assertion failures in
> > > some
> > > piglit tests, such as arb_program_interface_query-resource-query.
> > >
> > > That piglit test still fails on some tessellation related
> > > subtests.
> > > Specifically, these subtests fail:
> > >
> > > 'GL_PROGRAM_INPUT(tcs) active resources' expected 2 but got 3
> > > 'GL_PROGRAM_INPUT(tcs) max length name' expected 12 but got 16
> > > 'GL_PROGRAM_INPUT(tcs,tes) active resources' expected 2 but got 3
> > > 'GL_PROGRAM_INPUT(tcs,tes) max length name' expected 12 but got
> > > 16
> > > 'GL_PROGRAM_OUTPUT(tcs) active resources' expected 15 but got 3
> > > 'GL_PROGRAM_OUTPUT(tcs) max length name' expected 23 but got 12
> > >
> > > Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
> > > ---
> > > src/compiler/glsl/linker.cpp | 9 ++++++---
> > > 1 file changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/src/compiler/glsl/linker.cpp
> > > b/src/compiler/glsl/linker.cpp
> > > index 34b4a81..d277dd0 100644
> > > --- a/src/compiler/glsl/linker.cpp
> > > +++ b/src/compiler/glsl/linker.cpp
> > > @@ -4790,7 +4790,7 @@ link_shaders(struct gl_context *ctx, struct
> > > gl_shader_program *prog)
> > > */
> > > int next = last;
> > > for (int i = next - 1; i >= 0; i--) {
> > > - if (prog->_LinkedShaders[i] == NULL)
> > > + if (prog->_LinkedShaders[i] == NULL && i != 0)
> > Up to you but I think i != MESA_SHADER_VERTEX makes it slightly
> > easier
> > to follow.
> I'm a little bit torn. I thought about using MESA_SHADER_VERTEX, but
> that's not really the information that's important. What's important
> is
> always executing at the loop bound (which is 0) even when there's no
> shader there.
In that case I don't think this won't work as intended. The
_LinkedShaders array is defined as:
_LinkedShaders[MESA_SHADER_STAGES]
Here a zero index is the first GL stage not the first stage in the
program. So a program containing only geom and frag stages would hit
the same problem.
I think we need to do something like what is done with the last stage
above this code e.g.
if (first < MESA_SHADER_VERTEX && prog->SeparateShader) {
if (!assign_varying_locations(...))
goto done;
}
I wonder if that code should be calling check_against_output_limit()
also. I guess we probably have no SSO tests that exceed the limit.
>
> I think I'd want to either leave both i comparisons with zero or
> change
> both to MESA_SHADER_VERTEX. Do you have an opinion?
>
> >
> > Otherwise the series is Reviewed-by: Timothy Arceri
> > <timothy.arc...@collabora.com>
> >
> >
> > >
> > > continue;
> > >
> > > gl_shader *const sh_i = prog->_LinkedShaders[i];
> > > @@ -4806,8 +4806,11 @@ link_shaders(struct gl_context *ctx,
> > > struct
> > > gl_shader_program *prog)
> > > tfeedback_decls);
> > >
> > > /* This must be done after all dead varyings are
> > > eliminated. */
> > > - if (!check_against_output_limit(ctx, prog, sh_i))
> > > - goto done;
> > > + if (sh_i != NULL) {
> > > + if (!check_against_output_limit(ctx, prog, sh_i))
> > > {
> > > + goto done;
> > > + }
> > > + }
> > > if (!check_against_input_limit(ctx, prog, sh_next))
> > > goto done;
> > >
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
