[Mesa-dev] [PATCH 1/2] glsl: allow inputs in interface blocks in interpolateAt*

[Ian Romanick]

On 06/19/2017 04:45 AM, Nicolai Hähnle wrote:
> On 18.06.2017 21:18, Ian Romanick wrote:
>> On 06/16/2017 01:37 PM, Nicolai Hähnle wrote:
>>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>>
>>> Since interface blocks are simply considered groupings of input
>>> variables,
>>> this is allowed.
>>>
>>> var->data.must_be_shader_input is now determined on-the-fly after
>>> lowering
>>> interface blocks, since we don't want to disable varying packing for an
>>> entire block just because one input in it is used in interpolateAt*.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101378
>>
>> I looked at the bug, but I haven't looked at the piglit list... is there
>> a test for this?  We should add one.
> 
> I sent a series that adds more tests.
> 
> 
>> I /think/ this should also be tagged for stable.
> 
> Sure.
> 
> 
>>
>>> ---
>>>   src/compiler/glsl/ast_function.cpp                 |  7 +++++--
>>>   src/compiler/glsl/lower_named_interface_blocks.cpp | 18
>>> ++++++++++++++++++
>>>   2 files changed, 23 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/compiler/glsl/ast_function.cpp
>>> b/src/compiler/glsl/ast_function.cpp
>>> index 2d156ae..11897f7 100644
>>> --- a/src/compiler/glsl/ast_function.cpp
>>> +++ b/src/compiler/glsl/ast_function.cpp
>>> @@ -221,29 +221,32 @@ verify_parameter_modes(_mesa_glsl_parse_state
>>> *state,
>>>                                   formal->name);
>>>                  return false;
>>>               }
>>>               val = ((ir_swizzle *)val)->val;
>>>            }
>>>              while (val->ir_type == ir_type_dereference_array) {
>>>               val = ((ir_dereference_array *)val)->array;
>>>            }
>>>   +         if (const ir_dereference_record *deref =
>>> val->as_dereference_record()) {
>>> +            if (deref->record->type->is_interface())
>>> +               val = deref->record;
>>> +         }
>>> +
>>>            if (!val->as_dereference_variable() ||
>>>                val->variable_referenced()->data.mode !=
>>> ir_var_shader_in) {
>>>               _mesa_glsl_error(&loc, state,
>>>                                "parameter `%s` must be a shader input",
>>>                                formal->name);
>>>               return false;
>>>            }
>>> -
>>> -         val->variable_referenced()->data.must_be_shader_input = 1;
>>
>> I think I'd like to leave this here for non-interface blocks.  I could
>> envision someone changing the linker to not call
>> lower_named_interface_blocks for shaders that can't have interface
>> blocks... then things would break in hard to find ways.
> 
> Makes sense, changed locally.

With that change, this patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

I'll go take a look at the piglit tests too.