[Mesa-dev] [PATCH 5/6] glsl: teach opt_array_splitting about bindless images

[Samuel Pitoiset]

On 05/31/2017 11:34 AM, Timothy Arceri wrote:
> 
> 
> On 31/05/17 19:04, Samuel Pitoiset wrote:
>>
>>
>> On 05/31/2017 02:46 AM, Ian Romanick wrote:
>>> On 05/25/2017 11:07 AM, Samuel Pitoiset wrote:
>>>> Memory/format layout qualifiers shouldn't be lost when arrays
>>>> of images are splitted by this pass.
>>>>
>>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>>> ---
>>>>   src/compiler/glsl/opt_array_splitting.cpp | 17 +++++++++++++++--
>>>>   1 file changed, 15 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/compiler/glsl/opt_array_splitting.cpp 
>>>> b/src/compiler/glsl/opt_array_splitting.cpp
>>>> index e3073b022c..5841454574 100644
>>>> --- a/src/compiler/glsl/opt_array_splitting.cpp
>>>> +++ b/src/compiler/glsl/opt_array_splitting.cpp
>>>> @@ -449,9 +449,22 @@ optimize_split_arrays(exec_list *instructions, 
>>>> bool linked)
>>>>         for (unsigned int i = 0; i < entry->size; i++) {
>>>>            const char *name = ralloc_asprintf(mem_ctx, "%s_%d",
>>>>                                               entry->var->name, i);
>>>> -
>>>> -         entry->components[i] =
>>>> +         ir_variable *new_var =
>>>>               new(entry->mem_ctx) ir_variable(subtype, name, 
>>>> ir_var_temporary);
>>>> +
>>>> +         if (entry->var->type->contains_image()) {
>>>> +            /* Do not lost memory/format qualifiers when arrays of 
>>>> images are
>>>
>>>                           lose
>>>
>>>> +             * splitted.
>>>
>>>                    split
>>>
>>>> +             */
>>>> +            new_var->data.memory_read_only = 
>>>> entry->var->data.memory_read_only;
>>>> +            new_var->data.memory_write_only = 
>>>> entry->var->data.memory_write_only;
>>>> +            new_var->data.memory_coherent = 
>>>> entry->var->data.memory_coherent;
>>>> +            new_var->data.memory_volatile = 
>>>> entry->var->data.memory_volatile;
>>>> +            new_var->data.memory_restrict = 
>>>> entry->var->data.memory_restrict;
>>>> +            new_var->data.image_format = 
>>>> entry->var->data.image_format;
>>>
>>> Is there a reason to only copy these fields for images?  It seems like
>>> it should be safe to always do it.
>>
>> Which fields you are talking about? I only copy the image fields 
>> because I'm sure they have to be propagated when arrays are split. 
>> Maybe, some other fields should be propagated as well but that would 
>> require a deeper analysis.
> 
> I think he meant you could just drop the:
> 
>    if (entry->var->type->contains_image())
> 
> And just always copy the qualifiers as it should be harmless. Changing 
> it to without_array() or dropping entirely has my r-b so up to you.

Ah okay, yeah not a big deal.

> 
> 
>>
>>>
>>>> +         }
>>>> +
>>>> +         entry->components[i] = new_var;
>>>>            entry->var->insert_before(entry->components[i]);
>>>>         }
>>>>
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev