[Mesa-dev] [PATCH 4/5] glsl: Consider active all elements of a shared/std140 block array

[Iago Toral]

Besides fixing the mentioned dEQP crashes, this patch also generally
fixes instance arrays with UBOs. The problem we have now is that each
element in the UBO instance array is a separate UBO mapped to a specific
binding point (and thus, a separate buffer), but we kill the instances
that are not being referenced in the shader code, so if we have
something like this:

layout(std140, binding=2) uniform Fragments {
   vec4 v0;
   vec4 v1;
} inst[3];

And then the shader code only references inst[1], for example:

vec4 tfOutput0 = inst[1].v0;

That UBO read for inst[1].v0 can fail as a consequence of the fact that
we we are killing UBOs for inst[0] and inst[2] and we shouldn't.

I hit this while developing SSBO, which is the same thing, and this
patch fixes the problem.

Iago

On Wed, 2015-03-11 at 10:01 +0100, Eduardo Lima Mitev wrote:
> From: Antia Puentes <apuentes at igalia.com>
> 
> Commmit 1ca25ab (glsl: Do not eliminate 'shared' or 'std140'
> blocks or block members) considers active 'shared' and 'std140'
> uniform blocks and uniform block arrays but did not include the
> block array elements. It was possible to have an active uniform
> block array without any elements marked as used, making the
> assertion ((b->num_array_elements > 0) == b->type->is_array())
> in link_uniform_blocks fail.
> 
> Fixes the following 5 dEQP tests:
> 
>  * dEQP-GLES3.functional.ubo.random.nested_structs_instance_arrays.18
>  * dEQP-GLES3.functional.ubo.random.nested_structs_instance_arrays.24
>  * dEQP-GLES3.functional.ubo.random.nested_structs_arrays_instance_arrays.19
>  * dEQP-GLES3.functional.ubo.random.all_per_block_buffers.49
>  * dEQP-GLES3.functional.ubo.random.all_shared_buffer.36
> ---
>  src/glsl/link_uniform_block_active_visitor.cpp | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/src/glsl/link_uniform_block_active_visitor.cpp b/src/glsl/link_uniform_block_active_visitor.cpp
> index 292cde3..8379750 100644
> --- a/src/glsl/link_uniform_block_active_visitor.cpp
> +++ b/src/glsl/link_uniform_block_active_visitor.cpp
> @@ -105,6 +105,22 @@ link_uniform_block_active_visitor::visit(ir_variable *var)
>     assert(b->num_array_elements == 0);
>     assert(b->array_elements == NULL);
>     assert(b->type != NULL);
> +   assert(!b->type->is_array() || b->has_instance_name);
> +
> +   /* For uniform block arrays declared with a shared or std140 layout
> +    * qualifier, mark all its instances as used.
> +    */
> +   if (b->type->is_array() && b->type->length > 0) {
> +      b->num_array_elements = b->type->length;
> +      b->array_elements = reralloc(this->mem_ctx,
> +                                   b->array_elements,
> +                                   unsigned,
> +                                   b->num_array_elements);
> +
> +      for (unsigned i = 0; i < b->num_array_elements; i++) {
> +         b->array_elements[i] = i;
> +      }
> +   }
>  
>     return visit_continue;
>  }
> @@ -146,6 +162,13 @@ link_uniform_block_active_visitor::visit_enter(ir_dereference_array *ir)
>     assert((b->num_array_elements == 0) == (b->array_elements == NULL));
>     assert(b->type != NULL);
>  
> +   /* If the block array was declared with a shared or std140 layout qualifier,
> +    * all its instances have been already marked as used (see
> +    * link_uniform_block_active_visitor::visit(ir_variable *) function).
> +    */
> +   if (var->type->interface_packing == GLSL_INTERFACE_PACKING_PACKED)
> +      return visit_continue;
> +
>     ir_constant *c = ir->array_index->as_constant();
>  
>     if (c) {