[Mesa-dev] [PATCH 2/4] glsl/linker: fix location aliasing checks for interface variables

[Ilia Mirkin]

On Thu, Oct 19, 2017 at 5:35 PM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>
>
> On 20/10/17 08:27, Timothy Arceri wrote:
>>
>>
>>
>> On 20/10/17 08:19, Timothy Arceri wrote:
>>>
>>> On 20/10/17 04:21, Ilia Mirkin wrote:
>>>>
>>>> On Thu, Oct 19, 2017 at 12:45 PM, Ilia Mirkin <imirkin at alum.mit.edu>
>>>> wrote:
>>>>>
>>>>> On Thu, Oct 19, 2017 at 12:40 PM, Iago Toral <itoral at igalia.com> wrote:
>>>>>>
>>>>>> On Thu, 2017-10-19 at 12:37 -0400, Ilia Mirkin wrote:
>>>>>>>
>>>>>>> Will this work with SSO shaders? Presumably the validation still has
>>>>>>> to happen, but I don't think cross_validate_outputs_to_inputs() will
>>>>>>> end up getting called.
>>>>>>
>>>>>>
>>>>>> The piglit tests I mention use SSO so it seems to be working for this.
>>>>>> See for example:
>>>>>>
>>>>>> tests/spec/arb_enhanced_layouts/linker/block-member-locations/named-
>>>>>> block-member-location-overlap.shader_test
>>>>>
>>>>>
>>>>> Ah great. I'm a little curious how, since I don't see how
>>>>> cross_validate_outputs_to_inputs would get called for SSO shaders.
>>>>> Perhaps I'm looking at old code.
>>>>>
>>>>> Oh - because that test doesn't test SSO. It's missing a "SSO ENABLED"
>>>>> - can you try with that? It's just using a pipeline, but both shaders
>>>>> are ending up in it.
>>>>
>>>>
>>>> BTW, my solution to all this was
>>>>
>>>> https://patchwork.freedesktop.org/patch/175959/
>>>>
>>>> but Tim hated it, and I didn't have the time to properly respond.
>>>
>>>
>>> Hate is a strong word, the problem is it duplicated some of the
>>> checks/logic in cross_validate_outputs_to_inputs() unnecessarily. The checks
>>> should be pulled into a helper/helpers that can also be used by SSO.
>>
>>
>> Oh and your patch was also missing all the component checking logic which
>> we also should be doing for SSO. Moving the checks into helpers will give us
>> these check for free.
>
>
> Thinking about it some more, I don't see how your patch works for SSO.
>
> The only place you can validate the SSO pipeline is via
> _mesa_validate_program_pipeline() since we can't know what the other stages
> will be at link time.

You don't need to validate the pipeline. You need to make sure each
individual program links, and the code is called at glLinkShader()
time, at which point it will fail the link if the interfaces have
things they're not supposed to.