[Mesa-dev] [PATCH v2 36/82] glsl: add support for unsized arrays in shader storage blocks

[Timothy Arceri]

On Wed, 2015-06-03 at 09:01 +0200, Iago Toral Quiroga wrote:
> From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> 
> They only can be defined in the last position of the shader
> storage blocks.
> 
> When an unsized array is used in different shaders, it might be
> converted in different sized arrays, avoid get a linker error
> in that case.
> 
> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> ---
>  src/glsl/ast_array_index.cpp |   6 +--
>  src/glsl/ast_to_hir.cpp      |  32 +++++++++++++
>  src/glsl/ir.cpp              |   1 +
>  src/glsl/ir.h                |  14 ++++++
>  src/glsl/linker.cpp          | 107 ++++++++++++++++++++++++++++---------------
>  5 files changed, 121 insertions(+), 39 deletions(-)
> 
> diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
> index 752d86f..481efae 100644
> --- a/src/glsl/ast_array_index.cpp
> +++ b/src/glsl/ast_array_index.cpp
> @@ -106,7 +106,6 @@ update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE *loc,
>     }
>  }
>  
> -
>  ir_rvalue *
>  _mesa_ast_array_index_to_hir(void *mem_ctx,
>  			     struct _mesa_glsl_parse_state *state,
> @@ -182,8 +181,9 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,
>        if (array->type->is_array())
>           update_max_array_access(array, idx, &loc, state);
>     } else if (const_index == NULL && array->type->is_array()) {
> -      if (array->type->is_unsized_array()) {
> -	 _mesa_glsl_error(&loc, state, "unsized array index must be constant");
> +      if (array->type->is_unsized_array() &&
> +          array->variable_referenced()->data.mode != ir_var_shader_storage) {
> +            _mesa_glsl_error(&loc, state, "unsized array index must be constant");
>        } else if (array->type->fields.array->is_interface()
>                   && array->variable_referenced()->data.mode == ir_var_uniform
>                   && !state->is_version(400, 0) && !state->ARB_gpu_shader5_enable) {
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index e1e5ca9..3ec28dd 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -5507,6 +5507,19 @@ private:
>     bool found;
>  };
>  
> +static bool
> +is_unsized_array_last_element(ir_variable *v)
> +{
> +   const glsl_type *interface_type = v->get_interface_type();
> +   int length = interface_type->length;
> +
> +   assert(v->type->is_unsized_array());
> +
> +   /* Check if it is the last element of the interface */
> +   if (strcmp(interface_type->fields.structure[length-1].name, v->name) == 0)
> +      return true;
> +   return false;
> +}
>  
>  ir_rvalue *
>  ast_interface_block::hir(exec_list *instructions,
> @@ -5828,6 +5841,15 @@ ast_interface_block::hir(exec_list *instructions,
>           var->data.explicit_binding = this->layout.flags.q.explicit_binding;
>           var->data.binding = this->layout.binding;
>  
> +         if (var->is_in_shader_storage_block() && var->type->is_unsized_array()) {
> +            var->data.from_ssbo_unsized_array = true;
> +            if (!is_unsized_array_last_element(var)) {
> +               _mesa_glsl_error(&loc, state, "unsized array `%s' should be the"
> +                                 " last member of a shader storage block",
> +                                 var->name);
> +            }
> +         }

The existing comment above this is misleading I think its a copy and
paste error from the non instance name block code below.

Anyway to get to the point here you are testing if the block itself is
an unsized array not the member inside the block.

Also when gles both this test and the test below should throw an error
when there is an unsized array member but its not in a shader storage
block.

"If an array is declared as the last member of a shader storage block
and the size is not specified at compile-time, it is sized at run-time.
In all other cases,  arrays are sized only at compile-time."


> +
>           state->symbols->add_variable(var);
>           instructions->push_tail(var);
>        }
> @@ -5900,6 +5922,16 @@ ast_interface_block::hir(exec_list *instructions,
>           var->data.explicit_binding = this->layout.flags.q.explicit_binding;
>           var->data.binding = this->layout.binding;
>  
> +         if (var->data.mode == ir_var_shader_storage && var->type->is_unsized_array()) {
> +            var->data.from_ssbo_unsized_array = true;
> +            if (var->is_in_shader_storage_block() &&
> +                !is_unsized_array_last_element(var)) {
> +               _mesa_glsl_error(&loc, state, "unsized array `%s' should be the"
> +                                 " last member of a shader storage block",
> +                                 var->name);
> +            }
> +         }
> +
>           state->symbols->add_variable(var);
>           instructions->push_tail(var);
>        }