On 15/06/15 13:25, Timothy Arceri wrote:
> On Wed, 2015-06-03 at 09:01 +0200, Iago Toral Quiroga wrote:
>> From: Samuel Iglesias Gonsalvez <sigles...@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 <sigles...@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.
>
Yes, you are right. I am going to modify it.
> 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."
>
Good catch! I am going to fix it too.
Thanks for the review!
Sam
>
>> +
>> 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);
>> }
>
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
