On Wed, 2016-05-18 at 20:44 +1000, Timothy Arceri wrote:
> 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) {
Whoops that should be first > MESA_SHADER_VERTEX
> 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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
