[Mesa-dev] [PATCH v5 70/70] glsl: Mark as active all elements of shared/std140 block arrays

[Samuel Iglesias Gonsálvez]

On 21/09/15 13:11, Samuel Iglesias Gonsálvez wrote:
> On 21/09/15 09:41, Tapani Pälli wrote:
>> Seems like a nice fix, takes ES3 CTS failures from 116 to 64 on Haswell
>> (most failing tests are with ubos).
>>
>> Tested-by: Tapani Pälli <tapani.palli at intel.com>
>>
> 
> OK thanks!
> 
>> This is individual patch not related to just SSBOs, maybe this could be
>> pushed separately from the rest?
>>
> 
> Yes, this patch can be pushed separately from the rest of patches of the
> series: we just need an R-b, as usual.
> 
> We are going to discuss the proper solution with Timothy [0], as he
> found that we are not covering one case.
> 

Timothy has sent a patch fixing the packed case [0] and he developed a
piglit test for it [1].

We are going to wait for an R-b of Antía's patch before pushing it.

Sam

[0]
http://lists.freedesktop.org/archives/mesa-dev/2015-September/095070.html
[1] http://lists.freedesktop.org/archives/piglit/2015-September/017247.html

> Sam
> 
> [0] https://bugs.freedesktop.org/show_bug.cgi?id=83508
> 
> 
>> // Tapani
>>
>> On 09/10/2015 04:36 PM, Iago Toral Quiroga wrote:
>>> From: Antia Puentes <apuentes at igalia.com>
>>>
>>> Commit 1ca25ab (glsl: Do not eliminate 'shared' or 'std140' blocks
>>> or block members) considered as active 'shared' and 'std140' uniform
>>> blocks and uniform block arrays, but did not include the block array
>>> elements. Because of that, 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
>>>
>>> Fixes bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83508
>>> ---
>>>   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 5102947..fbe79de 100644
>>> --- a/src/glsl/link_uniform_block_active_visitor.cpp
>>> +++ b/src/glsl/link_uniform_block_active_visitor.cpp
>>> @@ -106,6 +106,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;
>>>   }
>>> @@ -147,6 +163,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 in link_uniform_block_active_visitor::visit(ir_variable
>>> *).
>>> +    */
>>> +   if (var->type->interface_packing != GLSL_INTERFACE_PACKING_PACKED)
>>> +      return visit_continue_with_parent;
>>> +
>>>      ir_constant *c = ir->array_index->as_constant();
>>>
>>>      if (c) {
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>