[Mesa-dev] [PATCH 1/6] glsl: handle memory qualifiers for struct with array of images

[Samuel Pitoiset]

On 05/31/2017 02:43 AM, Ian Romanick wrote:
> On 05/25/2017 11:07 AM, Samuel Pitoiset wrote:
>> This handles a situation like:
>>
>> struct {
>>     image2D imgs[6];
>> } s;
> 
> Without looking at the surrounding code... does this also handle cases like:
> 
> struct S { image2D imgs[6]; };
> struct   { S s } s;

Yes. With a shader like:

%%%%%
struct S { writeonly image2D imgs[6]; };
struct { S s; } s;

void main()
{
	imageStore(s.s.imgs[2], ivec2(0, 0), vec4(0, 0, 0, 0));
}
%%%%%

The generated GLSL IR looks like:

%%%%%
(
(declare (writeonly temporary ) image2D s_s_imgs_0)
(declare (writeonly temporary ) image2D s_s_imgs_1)
(declare (writeonly temporary ) image2D s_s_imgs_2)
(declare (writeonly temporary ) image2D s_s_imgs_3)
(declare (writeonly temporary ) image2D s_s_imgs_4)
(declare (writeonly temporary ) image2D s_s_imgs_5)
( function main
   (signature void
     (parameters
     )
     (
       (call __intrinsic_image_store  ((var_ref s_s_imgs_2) (constant 
ivec2 (0 0)) (constant vec4 (0.000000 0.000000 0.000000 0.000000)) ))

     ))

)

)
%%%%%

The 'writeonly' qualifier is correctly propagated.

> 
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/compiler/glsl/ast_to_hir.cpp | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
>> index 2dec8bbc32..65b6262f34 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -7402,9 +7402,10 @@ ast_process_struct_or_iface_block_members(exec_list *instructions,
>>            }
>>   
>>            /* Memory qualifiers are allowed on buffer and image variables, while
>> -          * the format qualifier is only accept for images.
>> +          * the format qualifier is only accepted for images.
>>             */
>> -         if (var_mode == ir_var_shader_storage || field_type->is_image()) {
>> +         if (var_mode == ir_var_shader_storage ||
>> +             field_type->without_array()->is_image()) {
>>               /* For readonly and writeonly qualifiers the field definition,
>>                * if set, overwrites the layout qualifier.
>>                */
>> @@ -7431,7 +7432,7 @@ ast_process_struct_or_iface_block_members(exec_list *instructions,
>>               fields[i].memory_restrict = qual->flags.q.restrict_flag ||
>>                                           (layout && layout->flags.q.restrict_flag);
>>   
>> -            if (field_type->is_image()) {
>> +            if (field_type->without_array()->is_image()) {
>>                  if (qual->flags.q.explicit_image_format) {
>>                     if (qual->image_base_type != field_type->sampled_type) {
>>                        _mesa_glsl_error(&loc, state, "format qualifier doesn't "
>>
>