[Mesa-dev] [PATCH 4/6] glsl/linker: don't fail non static used inputs without matching outputs

[Timothy Arceri]

On 6/2/19 1:11 am, Andres Gomez wrote:
> On Fri, 2019-02-01 at 18:37 -0500, Ilia Mirkin wrote:
>> On Fri, Feb 1, 2019 at 1:08 PM Andres Gomez <agomez at igalia.com> wrote:
>>> If there is no Static Use of an input variable, the linker shouldn't
>>> fail whenever there is no defined matching output variable in the
>>> previous stage.
>>>
>>>  From page 47 (page 51 of the PDF) of the GLSL 4.60 v.5 spec:
>>>
>>>    " Only the input variables that are statically read need to be
>>>      written by the previous stage; it is allowed to have superfluous
>>>      declarations of input variables."
>>>
>>> Now, we complete this exception whenever the input variable has an
>>> explicit location. Previously, 18004c338f6 ("glsl: fail when a
>>> shader's input var has not an equivalent out var in previous") took
>>> care of the cases in which the input variable didn't have an explicit
>>> location.
>>>
>>> Additionally, likewise 1aa5738e666 ("glsl: relax input->output
>>> validation for SSO programs"), avoid failing also for programs that
>>> utilize GL_ARB_separate_shader_objects.
>>>
>>> Cc: Timothy Arceri <tarceri at itsqueeze.com>
>>> Cc: Iago Toral Quiroga <itoral at igalia.com>
>>> Cc: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
>>> Cc: Tapani Pälli <tapani.palli at intel.com>
>>> Cc: Ian Romanick <ian.d.romanick at intel.com>
>>> Signed-off-by: Andres Gomez <agomez at igalia.com>
>>> ---
>>>   src/compiler/glsl/link_varyings.cpp | 16 ++++++++++++++--
>>>   1 file changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
>>> index e5f7d3e322a..6cebc5b3c5a 100644
>>> --- a/src/compiler/glsl/link_varyings.cpp
>>> +++ b/src/compiler/glsl/link_varyings.cpp
>>> @@ -808,8 +808,20 @@ cross_validate_outputs_to_inputs(struct gl_context *ctx,
>>>
>>>                  output = output_explicit_locations[idx][input->data.location_frac].var;
>>>
>>> -               if (output == NULL ||
>>> -                   input->data.location != output->data.location) {
>>> +               if (output == NULL) {
>>> +                  /* A linker failure should only happen when, for programs
>>> +                   * not using sso, there is no output declaration and there
>>> +                   * is Static Use of the declared input.
>>> +                   */
>>> +                  if (input->data.used && !prog->SeparateShader) {
>>
>> Should this differentiate whether this is the first stage of a
>> separable program vs a later one? Presumably the exception only
>> applies at the separate program boundary, not to each shader within
>> the program?
> 
> Cc-ing Tapani and Anuj.
> 
> As I understand the spec, it applies to every shader within the
> separable program.

No that's not what the spec says. From the quote below:

"With separable program objects, interfaces between shader stages 
may involve the outputs from one program object and the inputs from a 
second program object."

So if we have two "program" objects say:

1. vs->gs
2. fs

The relaxed restrictions only apply to the gs -> fs interface. Not the 
vs -> gs interface.


> 
>  From the ARB_separate_shader_objects spec:
> 
>    " With separable program objects, interfaces between shader stages
>      may involve the outputs from one program object and the inputs
>      from a second program object.  For such interfaces, it is not
>      possible to detect mismatches at link time, because the programs
>      are linked separately.  When each such program is linked, all
>      inputs or outputs interfacing with another program stage are
>      treated as active.  The linker will generate an executable that
>      assumes the presence of a compatible program on the other side of
>      the interface.  If a mismatch between programs occurs, no GL error
>      will be generated, but some or all of the inputs on the interface
>      will be undefined."
> 
> Notice that this has also been the interpretation from:
> 
> 1aa5738e666 ("glsl: relax input->output validation for SSO programs")

Yes this looks wrong also. We should write some piglit tests for this 
and fix it too.

> 
>>
>>> +                     linker_error(prog,
>>> +                                  "%s shader input `%s' with explicit location "
>>> +                                  "has no matching output\n",
>>> +                                  _mesa_shader_stage_to_string(consumer->Stage),
>>> +                                  input->name);
>>> +                     break;
>>> +                  }
>>> +               } else if (input->data.location != output->data.location) {
>>>                     linker_error(prog,
>>>                                  "%s shader input `%s' with explicit location "
>>>                                  "has no matching output\n",
>>> --
>>> 2.20.1
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev