[Mesa-dev] [PATCH 4/6] glsl: teach opt_structure_splitting about images in structures

[Timothy Arceri]

On 26/05/17 04:07, Samuel Pitoiset wrote:
> GL_ARB_bindless_texture allows images to be declared inside
> structures, but when memory/format qualifiers are used, they
> should be propagated when structures are splitted.
> 
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>   src/compiler/glsl/opt_structure_splitting.cpp | 23 +++++++++++++++++++++--
>   1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/src/compiler/glsl/opt_structure_splitting.cpp b/src/compiler/glsl/opt_structure_splitting.cpp
> index eac98b74cc..fb2120f5fe 100644
> --- a/src/compiler/glsl/opt_structure_splitting.cpp
> +++ b/src/compiler/glsl/opt_structure_splitting.cpp
> @@ -344,11 +344,30 @@ do_structure_splitting(exec_list *instructions)
>         for (unsigned int i = 0; i < entry->var->type->length; i++) {
>            const char *name = ralloc_asprintf(mem_ctx, "%s_%s", entry->var->name,
>                                               type->fields.structure[i].name);
> -
> -         entry->components[i] =
> +         ir_variable *new_var =
>               new(entry->mem_ctx) ir_variable(type->fields.structure[i].type,
>                                               name,
>                                               (ir_variable_mode) entry->var->data.mode);
> +
> +         if (type->fields.structure[i].type->contains_image()) {

Do we need to use contains_image() here rather than 
without_array()->is_image()?

Wouldn't contains_image() mean that for a struct that contained a struct 
which contained an image, we would enter the if unnecessary when 
splitting the outer struct (not to mention the needless recursion over 
nested struct in contains_image() itself).

If you agree with changing this to without_array()->is_image() then:

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>


> +            /* Do not lost memory/format qualifiers for images declared inside
> +             * structures as allowed by ARB_bindless_texture.
> +             */
> +            new_var->data.memory_read_only =
> +               type->fields.structure[i].memory_read_only;
> +            new_var->data.memory_write_only =
> +               type->fields.structure[i].memory_write_only;
> +            new_var->data.memory_coherent =
> +               type->fields.structure[i].memory_coherent;
> +            new_var->data.memory_volatile =
> +               type->fields.structure[i].memory_volatile;
> +            new_var->data.memory_restrict =
> +               type->fields.structure[i].memory_restrict;
> +            new_var->data.image_format =
> +               type->fields.structure[i].image_format;
> +         }
> +
> +         entry->components[i] = new_var;
>            entry->var->insert_before(entry->components[i]);
>         }
>   
>