Re: [Mesa-dev] [PATCH 25/24] glsl: add std140 layout support for AoA

[Timothy Arceri]

On Thu, 2015-09-24 at 11:31 +0200, Samuel Iglesias Gonsálvez wrote:
> 
> On 24/09/15 02:16, Timothy Arceri wrote:
> > On Wed, 2015-09-23 at 13:21 +0200, Samuel Iglesias Gonsálvez wrote:
> > > On 22/09/15 15:30, Samuel Iglesias Gonsálvez wrote:
> > > > Reviewed-by: Samuel Iglesias Gonsálvez 
> > > > 
> > > 
> > > Forgot to say that we need to implement a similar patch for
> > > std430_size() function, right?
> > 
> > Yeah I think so.
> > 
> > > 
> > > I will write a follow-up patch adding std430 support for AoA.
> > 
> > That would be great thanks.
> > 
> > I assume there will also be other places in the SSBO series that
> > might
> > need updates for AoA?
> > 
> 
> As I am not very familiar with AoA, I will need to check how AoA
> affects
> to SSBO-related code.
> 
> I plan to look at it soon and send follow-up patches (if needed). I
> might ask you to review my patches as you have implemented AoA.

Hi Samuel,

Have you started looking at this yet?

If not I might start looking at it tomorrow. I'm hoping its a case of
things mostly just working but at the least we may need a few extra
piglit tests.

Tim
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 25/24] glsl: add std140 layout support for AoA

[Samuel Iglesias Gonsálvez]

On 29/09/15 08:34, Timothy Arceri wrote:
> On Thu, 2015-09-24 at 11:31 +0200, Samuel Iglesias Gonsálvez wrote:
>>
>> On 24/09/15 02:16, Timothy Arceri wrote:
>>> On Wed, 2015-09-23 at 13:21 +0200, Samuel Iglesias Gonsálvez wrote:
 On 22/09/15 15:30, Samuel Iglesias Gonsálvez wrote:
> Reviewed-by: Samuel Iglesias Gonsálvez 
>

 Forgot to say that we need to implement a similar patch for
 std430_size() function, right?
>>>
>>> Yeah I think so.
>>>

 I will write a follow-up patch adding std430 support for AoA.
>>>
>>> That would be great thanks.
>>>
>>> I assume there will also be other places in the SSBO series that
>>> might
>>> need updates for AoA?
>>>
>>
>> As I am not very familiar with AoA, I will need to check how AoA
>> affects
>> to SSBO-related code.
>>
>> I plan to look at it soon and send follow-up patches (if needed). I
>> might ask you to review my patches as you have implemented AoA.
> 
> Hi Samuel,
> 
> Have you started looking at this yet?

Yes, I have a couple of patches that seem to work. Changes seem to be
trivial to SSBO load/stores/unsized-array-length-calculation but I would
like to test with complex variable types (matNxM, structures...).

I plan to do it along today.

> 
> If not I might start looking at it tomorrow. I'm hoping its a case of
> things mostly just working but at the least we may need a few extra
> piglit tests.
> 

OK, my idea is to send the patches today afternoon or tomorrow.

Thanks!

Sam
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 25/24] glsl: add std140 layout support for AoA

[Samuel Iglesias Gonsálvez]

On 24/09/15 02:16, Timothy Arceri wrote:
> On Wed, 2015-09-23 at 13:21 +0200, Samuel Iglesias Gonsálvez wrote:
>> On 22/09/15 15:30, Samuel Iglesias Gonsálvez wrote:
>>> Reviewed-by: Samuel Iglesias Gonsálvez 
>>>
>>
>> Forgot to say that we need to implement a similar patch for
>> std430_size() function, right?
> 
> Yeah I think so.
> 
>>
>> I will write a follow-up patch adding std430 support for AoA.
> 
> That would be great thanks.
> 
> I assume there will also be other places in the SSBO series that might
> need updates for AoA?
> 

As I am not very familiar with AoA, I will need to check how AoA affects
to SSBO-related code.

I plan to look at it soon and send follow-up patches (if needed). I
might ask you to review my patches as you have implemented AoA.

Thanks,

Sam

> 
>>
>> Sam
>>
>>> On 20/09/15 14:07, Timothy Arceri wrote:
 ---
  I noticed this problem after adding AoA support [1] to Ian's
 random UBO test
  script [2].

  [1] http://patchwork.freedesktop.org/patch/59956/
  [2] http://cgit.freedesktop.org/~idr/piglit/log/?h=ubo-lolz

  src/glsl/glsl_types.cpp | 13 +++--
  1 file changed, 7 insertions(+), 6 deletions(-)

 diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
 index 86f0ea5..952bd0a 100644
 --- a/src/glsl/glsl_types.cpp
 +++ b/src/glsl/glsl_types.cpp
 @@ -1310,8 +1310,8 @@ glsl_type::std140_size(bool row_major)
 const
unsigned int array_len;
  
if (this->is_array()) {
 -   element_type = this->fields.array;
 -   array_len = this->length;
 +   element_type = this->without_array();
 +   array_len = this->arrays_of_arrays_size();
} else {
 element_type = this;
 array_len = 1;
 @@ -1344,12 +1344,13 @@ glsl_type::std140_size(bool row_major)
 const
  *  the array are laid out in order, according to rule
 (9).
  */
 if (this->is_array()) {
 -  if (this->fields.array->is_record()) {
 -   return this->length * this->fields.array
 ->std140_size(row_major);
 +  if (this->without_array()->is_record()) {
 +   return this->arrays_of_arrays_size() *
 +this->without_array()->std140_size(row_major);
} else {
 unsigned element_base_align =
 -  this->fields.array
 ->std140_base_alignment(row_major);
 -   return this->length * MAX2(element_base_align, 16);
 +  this->without_array()
 ->std140_base_alignment(row_major);
 +   return this->arrays_of_arrays_size() *
 MAX2(element_base_align, 16);
}
 }
  

> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 25/24] glsl: add std140 layout support for AoA

[Samuel Iglesias Gonsálvez]

On 22/09/15 15:30, Samuel Iglesias Gonsálvez wrote:
> Reviewed-by: Samuel Iglesias Gonsálvez 
>

Forgot to say that we need to implement a similar patch for
std430_size() function, right?

I will write a follow-up patch adding std430 support for AoA.

Sam

> On 20/09/15 14:07, Timothy Arceri wrote:
>> ---
>>  I noticed this problem after adding AoA support [1] to Ian's random UBO test
>>  script [2].
>>
>>  [1] http://patchwork.freedesktop.org/patch/59956/
>>  [2] http://cgit.freedesktop.org/~idr/piglit/log/?h=ubo-lolz
>>
>>  src/glsl/glsl_types.cpp | 13 +++--
>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
>> index 86f0ea5..952bd0a 100644
>> --- a/src/glsl/glsl_types.cpp
>> +++ b/src/glsl/glsl_types.cpp
>> @@ -1310,8 +1310,8 @@ glsl_type::std140_size(bool row_major) const
>>unsigned int array_len;
>>  
>>if (this->is_array()) {
>> - element_type = this->fields.array;
>> - array_len = this->length;
>> + element_type = this->without_array();
>> + array_len = this->arrays_of_arrays_size();
>>} else {
>>   element_type = this;
>>   array_len = 1;
>> @@ -1344,12 +1344,13 @@ glsl_type::std140_size(bool row_major) const
>>  *  the array are laid out in order, according to rule (9).
>>  */
>> if (this->is_array()) {
>> -  if (this->fields.array->is_record()) {
>> - return this->length * this->fields.array->std140_size(row_major);
>> +  if (this->without_array()->is_record()) {
>> + return this->arrays_of_arrays_size() *
>> +this->without_array()->std140_size(row_major);
>>} else {
>>   unsigned element_base_align =
>> -this->fields.array->std140_base_alignment(row_major);
>> - return this->length * MAX2(element_base_align, 16);
>> +this->without_array()->std140_base_alignment(row_major);
>> + return this->arrays_of_arrays_size() * MAX2(element_base_align, 16);
>>}
>> }
>>  
>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 25/24] glsl: add std140 layout support for AoA

[Timothy Arceri]

On Wed, 2015-09-23 at 13:21 +0200, Samuel Iglesias Gonsálvez wrote:
> On 22/09/15 15:30, Samuel Iglesias Gonsálvez wrote:
> > Reviewed-by: Samuel Iglesias Gonsálvez 
> > 
> 
> Forgot to say that we need to implement a similar patch for
> std430_size() function, right?

Yeah I think so.

> 
> I will write a follow-up patch adding std430 support for AoA.

That would be great thanks.

I assume there will also be other places in the SSBO series that might
need updates for AoA?


> 
> Sam
> 
> > On 20/09/15 14:07, Timothy Arceri wrote:
> > > ---
> > >  I noticed this problem after adding AoA support [1] to Ian's
> > > random UBO test
> > >  script [2].
> > > 
> > >  [1] http://patchwork.freedesktop.org/patch/59956/
> > >  [2] http://cgit.freedesktop.org/~idr/piglit/log/?h=ubo-lolz
> > > 
> > >  src/glsl/glsl_types.cpp | 13 +++--
> > >  1 file changed, 7 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> > > index 86f0ea5..952bd0a 100644
> > > --- a/src/glsl/glsl_types.cpp
> > > +++ b/src/glsl/glsl_types.cpp
> > > @@ -1310,8 +1310,8 @@ glsl_type::std140_size(bool row_major)
> > > const
> > >unsigned int array_len;
> > >  
> > >if (this->is_array()) {
> > > -  element_type = this->fields.array;
> > > -  array_len = this->length;
> > > +  element_type = this->without_array();
> > > +  array_len = this->arrays_of_arrays_size();
> > >} else {
> > >element_type = this;
> > >array_len = 1;
> > > @@ -1344,12 +1344,13 @@ glsl_type::std140_size(bool row_major)
> > > const
> > >  *  the array are laid out in order, according to rule
> > > (9).
> > >  */
> > > if (this->is_array()) {
> > > -  if (this->fields.array->is_record()) {
> > > -  return this->length * this->fields.array
> > > ->std140_size(row_major);
> > > +  if (this->without_array()->is_record()) {
> > > +  return this->arrays_of_arrays_size() *
> > > +this->without_array()->std140_size(row_major);
> > >} else {
> > >unsigned element_base_align =
> > > - this->fields.array
> > > ->std140_base_alignment(row_major);
> > > -  return this->length * MAX2(element_base_align, 16);
> > > + this->without_array()
> > > ->std140_base_alignment(row_major);
> > > +  return this->arrays_of_arrays_size() *
> > > MAX2(element_base_align, 16);
> > >}
> > > }
> > >  
> > > 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 25/24] glsl: add std140 layout support for AoA

[Samuel Iglesias Gonsálvez]

Reviewed-by: Samuel Iglesias Gonsálvez 

On 20/09/15 14:07, Timothy Arceri wrote:
> ---
>  I noticed this problem after adding AoA support [1] to Ian's random UBO test
>  script [2].
> 
>  [1] http://patchwork.freedesktop.org/patch/59956/
>  [2] http://cgit.freedesktop.org/~idr/piglit/log/?h=ubo-lolz
> 
>  src/glsl/glsl_types.cpp | 13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 86f0ea5..952bd0a 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -1310,8 +1310,8 @@ glsl_type::std140_size(bool row_major) const
>unsigned int array_len;
>  
>if (this->is_array()) {
> -  element_type = this->fields.array;
> -  array_len = this->length;
> +  element_type = this->without_array();
> +  array_len = this->arrays_of_arrays_size();
>} else {
>element_type = this;
>array_len = 1;
> @@ -1344,12 +1344,13 @@ glsl_type::std140_size(bool row_major) const
>  *  the array are laid out in order, according to rule (9).
>  */
> if (this->is_array()) {
> -  if (this->fields.array->is_record()) {
> -  return this->length * this->fields.array->std140_size(row_major);
> +  if (this->without_array()->is_record()) {
> +  return this->arrays_of_arrays_size() *
> +this->without_array()->std140_size(row_major);
>} else {
>unsigned element_base_align =
> - this->fields.array->std140_base_alignment(row_major);
> -  return this->length * MAX2(element_base_align, 16);
> + this->without_array()->std140_base_alignment(row_major);
> +  return this->arrays_of_arrays_size() * MAX2(element_base_align, 16);
>}
> }
>  
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 25/24] glsl: add std140 layout support for AoA

[Timothy Arceri]

---
 I noticed this problem after adding AoA support [1] to Ian's random UBO test
 script [2].

 [1] http://patchwork.freedesktop.org/patch/59956/
 [2] http://cgit.freedesktop.org/~idr/piglit/log/?h=ubo-lolz

 src/glsl/glsl_types.cpp | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index 86f0ea5..952bd0a 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -1310,8 +1310,8 @@ glsl_type::std140_size(bool row_major) const
   unsigned int array_len;
 
   if (this->is_array()) {
-element_type = this->fields.array;
-array_len = this->length;
+element_type = this->without_array();
+array_len = this->arrays_of_arrays_size();
   } else {
 element_type = this;
 array_len = 1;
@@ -1344,12 +1344,13 @@ glsl_type::std140_size(bool row_major) const
 *  the array are laid out in order, according to rule (9).
 */
if (this->is_array()) {
-  if (this->fields.array->is_record()) {
-return this->length * this->fields.array->std140_size(row_major);
+  if (this->without_array()->is_record()) {
+return this->arrays_of_arrays_size() *
+this->without_array()->std140_size(row_major);
   } else {
 unsigned element_base_align =
-   this->fields.array->std140_base_alignment(row_major);
-return this->length * MAX2(element_base_align, 16);
+   this->without_array()->std140_base_alignment(row_major);
+return this->arrays_of_arrays_size() * MAX2(element_base_align, 16);
   }
}
 
-- 
2.4.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev