On 2015-07-14 00:46:10, 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.
>
> v2:
> - Rework error condition and error messages (Timothy Arteri)
Arceri
> Signed-off-by: Samuel Iglesias Gonsalvez <sigles...@igalia.com>
> ---
> src/glsl/ast_array_index.cpp | 5 +-
> src/glsl/ast_to_hir.cpp | 66 ++++++++++++++++++++++++++
> src/glsl/ir.cpp | 1 +
> src/glsl/ir.h | 14 ++++++
> src/glsl/linker.cpp | 107
> ++++++++++++++++++++++++++++---------------
> 5 files changed, 155 insertions(+), 38 deletions(-)
>
> diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
> index 2c79002..8a1ae67 100644
> --- a/src/glsl/ast_array_index.cpp
> +++ b/src/glsl/ast_array_index.cpp
> @@ -182,8 +182,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 ca30dbc..1b4ee22 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -5599,6 +5599,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,
> @@ -5913,6 +5926,33 @@ ast_interface_block::hir(exec_list *instructions,
> if (state->stage == MESA_SHADER_GEOMETRY && var_mode ==
> ir_var_shader_in)
> handle_geometry_shader_input_decl(state, loc, var);
>
> + for (unsigned i = 0; i < num_variables; i++) {
> + if (fields[i].type->is_unsized_array()) {
> + if (var_mode == ir_var_shader_storage) {
> + if (i != (num_variables - 1)) {
> + _mesa_glsl_error(&loc, state, "unsized array `%s'
> definition: "
> + "only last member of a shader storage
> block "
> + "can be defined as unsized array",
> + fields[i].name);
> + }
> + } else {
> + /* From GLSL ES 3.10 spec, section 4.1.9 "Arrays":
> + *
> + * "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."
> + */
Where is the check for 'last member' in the es path?
Is this es check new? If so, should the es check be added in a patch
before this one, and then extended for SSBO support?
> + if (state->es_shader) {
> + _mesa_glsl_error(&loc, state, "unsized array `%s'
> definition: "
> + "only last member of a shader storage block
> "
> + "can be defined as unsized array",
> + fields[i].name);
> + }
> + }
> + }
> + }
> +
>
> if (ir_variable *earlier =
> state->symbols->get_variable(this->instance_name)) {
> if (!redeclaring_per_vertex) {
> @@ -6003,6 +6043,32 @@ 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->type->is_unsized_array()) {
> + if (var->is_in_shader_storage_block()) {
> + if (!is_unsized_array_last_element(var)) {
> + _mesa_glsl_error(&loc, state, "unsized array `%s'
> definition: "
> + "only last member of a shader storage
> block "
> + "can be defined as unsized array",
> + var->name);
> + }
> + var->data.from_ssbo_unsized_array = true;
> + } else {
> + /* From GLSL ES 3.10 spec, section 4.1.9 "Arrays":
> + *
> + * "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."
> + */
> + if (state->es_shader) {
> + _mesa_glsl_error(&loc, state, "unsized array `%s'
> definition: "
> + "only last member of a shader storage block
> "
> + "can be defined as unsized array",
> + var->name);
> + }
> + }
> + }
> +
> state->symbols->add_variable(var);
> instructions->push_tail(var);
> }
> diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
> index 9a25bf4..390e8f3 100644
> --- a/src/glsl/ir.cpp
> +++ b/src/glsl/ir.cpp
> @@ -1654,6 +1654,7 @@ ir_variable::ir_variable(const struct glsl_type *type,
> const char *name,
> this->data.image_coherent = false;
> this->data.image_volatile = false;
> this->data.image_restrict = false;
> + this->data.from_ssbo_unsized_array = false;
>
> if (type != NULL) {
> if (type->base_type == GLSL_TYPE_SAMPLER)
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index 1c7829b..e677b89 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -453,6 +453,15 @@ public:
> }
>
> /**
> + * Determine whether or not a variable is part of a shader storage block.
> + */
> + inline bool is_in_shader_storage_block() const
> + {
> + return this->data.mode == ir_var_shader_storage &&
> + this->interface_type != NULL;
> + }
> +
> + /**
> * Determine whether or not a variable is the declaration of an interface
> * block
> *
> @@ -777,6 +786,11 @@ public:
> unsigned image_restrict:1;
>
> /**
> + * ARB_shader_storage_buffer_object
> + */
> + unsigned from_ssbo_unsized_array:1; /**< unsized array buffer
> variable. */
> +
> + /**
> * Emit a warning if this variable is accessed.
> */
> private:
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 28ad2f3..ed490d0 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -724,30 +724,40 @@ validate_intrastage_arrays(struct gl_shader_program
> *prog,
> * In addition, set the type of the linked variable to the
> * explicitly sized array.
> */
> - if (var->type->is_array() && existing->type->is_array() &&
> - (var->type->fields.array == existing->type->fields.array) &&
> - ((var->type->length == 0)|| (existing->type->length == 0))) {
> - if (var->type->length != 0) {
> - if (var->type->length <= existing->data.max_array_access) {
> - linker_error(prog, "%s `%s' declared as type "
> - "`%s' but outermost dimension has an index"
> - " of `%i'\n",
> - mode_string(var),
> - var->name, var->type->name,
> - existing->data.max_array_access);
> - }
> - existing->type = var->type;
> - return true;
> - } else if (existing->type->length != 0) {
> - if(existing->type->length <= var->data.max_array_access) {
> - linker_error(prog, "%s `%s' declared as type "
> - "`%s' but outermost dimension has an index"
> - " of `%i'\n",
> - mode_string(var),
> - var->name, existing->type->name,
> - var->data.max_array_access);
> + if (var->type->is_array() && existing->type->is_array()) {
> + if ((var->type->fields.array == existing->type->fields.array) &&
> + ((var->type->length == 0)|| (existing->type->length == 0))) {
> + if (var->type->length != 0) {
> + if (var->type->length <= existing->data.max_array_access) {
> + linker_error(prog, "%s `%s' declared as type "
> + "`%s' but outermost dimension has an index"
> + " of `%i'\n",
> + mode_string(var),
> + var->name, var->type->name,
> + existing->data.max_array_access);
> + }
> + existing->type = var->type;
> + return true;
> + } else if (existing->type->length != 0) {
> + if(existing->type->length <= var->data.max_array_access &&
> + !existing->data.from_ssbo_unsized_array) {
> + linker_error(prog, "%s `%s' declared as type "
> + "`%s' but outermost dimension has an index"
> + " of `%i'\n",
> + mode_string(var),
> + var->name, existing->type->name,
> + var->data.max_array_access);
> + }
> + return true;
> }
> - return true;
> + } else {
> + /* The arrays of structs could have different glsl_type pointers but
> + * they are actually the same type. Use record_compare() to check
> that.
> + */
> + if (existing->type->fields.array->is_record() &&
> + var->type->fields.array->is_record() &&
> +
> existing->type->fields.array->record_compare(var->type->fields.array))
> + return true;
> }
> }
> return false;
> @@ -802,12 +812,24 @@ cross_validate_globals(struct gl_shader_program *prog,
> && existing->type->record_compare(var->type)) {
> existing->type = var->type;
> } else {
Looks like you could use else if.
> - linker_error(prog, "%s `%s' declared as type "
> - "`%s' and type `%s'\n",
> - mode_string(var),
> - var->name, var->type->name,
> - existing->type->name);
> - return;
> + /* If it is an unsized array in a Shader Storage Block,
> + * two different shaders can access to different
> elements.
> + * Because of that, they might be converted to different
> + * sized arrays, then check that they are compatible but
> + * forget about the array size.
> + */
How about "ignore" in place of "forget about"?
-Jordan
> + if (!(var->data.mode == ir_var_shader_storage &&
> + var->data.from_ssbo_unsized_array &&
> + existing->data.mode == ir_var_shader_storage &&
> + existing->data.from_ssbo_unsized_array &&
> + var->type->gl_type == existing->type->gl_type)) {
> + linker_error(prog, "%s `%s' declared as type "
> + "`%s' and type `%s'\n",
> + mode_string(var),
> + var->name, var->type->name,
> + existing->type->name);
> + return;
> + }
> }
> }
> }
> @@ -1234,12 +1256,14 @@ public:
>
> virtual ir_visitor_status visit(ir_variable *var)
> {
> - fixup_type(&var->type, var->data.max_array_access);
> + fixup_type(&var->type, var->data.max_array_access,
> + var->data.from_ssbo_unsized_array);
> if (var->type->is_interface()) {
> if (interface_contains_unsized_arrays(var->type)) {
> const glsl_type *new_type =
> resize_interface_members(var->type,
> - var->get_max_ifc_array_access());
> + var->get_max_ifc_array_access(),
> + var->is_in_shader_storage_block());
> var->type = new_type;
> var->change_interface_type(new_type);
> }
> @@ -1248,7 +1272,8 @@ public:
> if (interface_contains_unsized_arrays(var->type->fields.array)) {
> const glsl_type *new_type =
> resize_interface_members(var->type->fields.array,
> - var->get_max_ifc_array_access());
> + var->get_max_ifc_array_access(),
> + var->is_in_shader_storage_block());
> var->change_interface_type(new_type);
> var->type = update_interface_members_array(var->type, new_type);
> }
> @@ -1289,9 +1314,10 @@ private:
> * If the type pointed to by \c type represents an unsized array, replace
> * it with a sized array whose size is determined by max_array_access.
> */
> - static void fixup_type(const glsl_type **type, unsigned max_array_access)
> + static void fixup_type(const glsl_type **type, unsigned max_array_access,
> + bool from_ssbo_unsized_array)
> {
> - if ((*type)->is_unsized_array()) {
> + if (!from_ssbo_unsized_array && (*type)->is_unsized_array()) {
> *type = glsl_type::get_array_instance((*type)->fields.array,
> max_array_access + 1);
> assert(*type != NULL);
> @@ -1334,14 +1360,23 @@ private:
> */
> static const glsl_type *
> resize_interface_members(const glsl_type *type,
> - const unsigned *max_ifc_array_access)
> + const unsigned *max_ifc_array_access,
> + bool is_ssbo)
> {
> unsigned num_fields = type->length;
> glsl_struct_field *fields = new glsl_struct_field[num_fields];
> memcpy(fields, type->fields.structure,
> num_fields * sizeof(*fields));
> for (unsigned i = 0; i < num_fields; i++) {
> - fixup_type(&fields[i].type, max_ifc_array_access[i]);
> + /* If SSBO last member is unsized array, we don't replace it by a
> sized
> + * array.
> + */
> + if (is_ssbo && i == (num_fields - 1))
> + fixup_type(&fields[i].type, max_ifc_array_access[i],
> + true);
> + else
> + fixup_type(&fields[i].type, max_ifc_array_access[i],
> + false);
> }
> glsl_interface_packing packing =
> (glsl_interface_packing) type->interface_packing;
> --
> 1.9.1
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
