[Mesa-dev] [PATCH] glsl: Use correct mode for split components.

[Dave Airlie]

On 19 May 2016 at 13:18, Jordan Justen <jordan.l.justen at intel.com> wrote:
> On 2016-04-25 16:39:21, Kenneth Graunke wrote:
>> On Wednesday, April 20, 2016 3:42:01 PM PDT Bas Nieuwenhuizen wrote:
>> > The mode should stay the same as the original struct. In
>> > particular, shared should not be changed to temporary.
>> >
>> > Signed-off-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
>> > ---
>> >  src/compiler/glsl/opt_structure_splitting.cpp | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/src/compiler/glsl/opt_structure_splitting.cpp b/src/compiler/
>> glsl/opt_structure_splitting.cpp
>> > index 0d18a2f..f4c129e 100644
>> > --- a/src/compiler/glsl/opt_structure_splitting.cpp
>> > +++ b/src/compiler/glsl/opt_structure_splitting.cpp
>> > @@ -351,7 +351,7 @@ do_structure_splitting(exec_list *instructions)
>> >        entry->components[i] =
>> >           new(entry->mem_ctx) ir_variable(type->fields.structure[i].type,
>> >                                           name,
>> > -                                         ir_var_temporary);
>> > +                                         (ir_variable_mode) entry->var->data.mode);
>> >        entry->var->insert_before(entry->components[i]);
>> >        }
>> >
>> >
>>
>> ir_structure_reference_visitor::get_variable_entry bails for uniforms,
>> shader inputs/outputs, and SSBOs.  I believe the intention was to bail
>> on anything other than auto/temporary variables.  Presumably, we just
>> forgot to disallow shared here.
>>
>> I think the primary reason for doing that was because uniforms/SSBOs are
>> visible via API introspection, and inputs/outputs needed has cross-stage
>> implications.
>>
>> I don't know much about shared variables, but as long as they're not
>> visible via the API, they're probably safe to split.  If so, this patch
>> seems reasonable.
>>
>
> The layout of shared variables is not visible to the API. I don't know
> of a reason to disable this pass, so we should probably add this
> patch.
>
> Is piglit okay with the patch? Does it fix any applications?

Yes it fixes UE4 with GL4.3 enabled on radeonsi. I haven't seen any piglit
regressions due to it.

Dave.