[Mesa-dev] [PATCH] nir: fix compacting varyings when XFB outputs are present

[Samuel Pitoiset]

On 10/10/18 10:34 AM, Timothy Arceri wrote:
> On 10/10/18 7:25 pm, Samuel Pitoiset wrote:
>> On 10/10/18 10:14 AM, Timothy Arceri wrote:
>>> On 10/10/18 6:44 pm, Samuel Pitoiset wrote:
>>>> We shouldn't try to compact any varyings known as always
>>>> active IO, especially XFB outputs. For example, if one
>>>> component of an xfb output is also used as input varying
>>>> in the next stage, it shouldn't be compacted.
>>>>
>>>> Because we look at the input varyings from the consumer
>>>> stage, we don't know if one of them is an XFB output. One
>>>> solution is to mark all components as used when
>>>> always_active_io is true to avoid wrong remapping.
>>>>
>>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>>> ---
>>>>   src/compiler/nir/nir_linking_helpers.c | 9 +++++++++
>>>>   1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/src/compiler/nir/nir_linking_helpers.c 
>>>> b/src/compiler/nir/nir_linking_helpers.c
>>>> index 85712a7cb1..88014e9a1d 100644
>>>> --- a/src/compiler/nir/nir_linking_helpers.c
>>>> +++ b/src/compiler/nir/nir_linking_helpers.c
>>>> @@ -236,6 +236,15 @@ 
>>>> get_slot_component_masks_and_interp_types(struct exec_list *var_list,
>>>>                  get_interp_type(var, default_to_smooth_interp);
>>>>               interp_loc[location + i] = get_interp_loc(var);
>>>> +            if (var->data.always_active_io) {
>>>> +               /* Mark all components as used to avoid repacting 
>>>> xfb varyings
>>>> +                * wrongly. For instance, if one component of an xfb 
>>>> output is
>>>> +                * also used as input varying in the next stage.
>>>> +                */
>>>> +               comps[location + i] |= 0xf;
>>>> +               continue;
>>>> +            }
>>>
>>> In GLSL we specifically mark the corresponding inputs as 
>>> always_active_io. I think we should try to do that for spirv also 
>>> otherwise we are going to run into issues with the other lowering 
>>> passes that check always_active_io such as scalar/array lowering.
>>
>> https://cgit.freedesktop.org/mesa/mesa/commit/?id=a1bc1523408a305b14a8c297ea93a28bb12cee5d 
>>
>>
>> Should be enough?
> 
> I haven't read the spec but if its the same as GLSL that probably only 
> sets it on outputs.

How does the GLSL compiler handle that then? Does it somehow propagate 
always_active_io to the input varyings of the next stage?

> 
>>
>>>
>>>> +
>>>>               if (dual_slot) {
>>>>                  if (i & 1) {
>>>>                     comps[location + i] |= ((1 << comps_slot2) - 1);
>>>>