[Mesa-dev] [PATCH 8/8] glsl/linker: Ensure the first stage of an SSO pipeline has input locs assigned

[Ian Romanick]

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.romanick at 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.romanick at 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.

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.arceri at 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;
>>  
>