Re: [Mesa-dev] [PATCH 2/2] st/glsl_to_tgsi: fix block copies of arrays of structs

2016-10-18 Thread Marek Olšák
For the series:

Reviewed-by: Marek Olšák 

Marek

On Mon, Oct 17, 2016 at 7:25 PM, Nicolai Hähnle  wrote:
> From: Nicolai Hähnle 
>
> Use a full writemask in this case. This is relevant e.g. when a function
> has an inout argument which is an array of structs.
> ---
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 1662f7f..b91ebaf 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -2964,24 +2964,26 @@ glsl_to_tgsi_visitor::visit(ir_assignment *ir)
>
>   if (variable->data.location == FRAG_RESULT_DEPTH)
>  l.writemask = WRITEMASK_Z;
>   else {
>  assert(variable->data.location == FRAG_RESULT_STENCIL);
>  l.writemask = WRITEMASK_Y;
>   }
>} else if (ir->write_mask == 0) {
>   assert(!ir->lhs->type->is_scalar() && !ir->lhs->type->is_vector());
>
> - if (ir->lhs->type->is_array() || ir->lhs->type->is_matrix()) {
> -unsigned num_elements = 
> ir->lhs->type->without_array()->vector_elements;
> + unsigned num_elements = 
> ir->lhs->type->without_array()->vector_elements;
> +
> + if (num_elements) {
>  l.writemask = u_bit_consecutive(0, num_elements);
>   } else {
> +// The type is a struct or an array of (array of) structs.
>  l.writemask = WRITEMASK_XYZW;
>   }
>} else {
>   l.writemask = ir->write_mask;
>}
>
>for (int i = 0; i < 4; i++) {
>   if (l.writemask & (1 << i)) {
>  first_enabled_chan = GET_SWZ(r.swizzle, i);
>  break;
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] st/glsl_to_tgsi: fix block copies of arrays of structs

2016-10-17 Thread Nicolai Hähnle

On 17.10.2016 23:08, Timothy Arceri wrote:

On Mon, 2016-10-17 at 19:25 +0200, Nicolai Hähnle wrote:

From: Nicolai Hähnle 

Use a full writemask in this case. This is relevant e.g. when a
function
has an inout argument which is an array of structs.
---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 1662f7f..b91ebaf 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -2964,24 +2964,26 @@ glsl_to_tgsi_visitor::visit(ir_assignment
*ir)

  if (variable->data.location == FRAG_RESULT_DEPTH)
 l.writemask = WRITEMASK_Z;
  else {
 assert(variable->data.location == FRAG_RESULT_STENCIL);
 l.writemask = WRITEMASK_Y;
  }
   } else if (ir->write_mask == 0) {
  assert(!ir->lhs->type->is_scalar() && !ir->lhs->type-

is_vector());


- if (ir->lhs->type->is_array() || ir->lhs->type-

is_matrix()) {

-unsigned num_elements = ir->lhs->type->without_array()-

vector_elements;

+ unsigned num_elements = ir->lhs->type->without_array()-

vector_elements;

+
+ if (num_elements) {
 l.writemask = u_bit_consecutive(0, num_elements);
  } else {
+// The type is a struct or an array of (array of)
structs.



C++ style comment in Mesa :(


The dangers of code-base-hopping. I'll fix that of course, thanks!

Nicolai





 l.writemask = WRITEMASK_XYZW;
  }
   } else {
  l.writemask = ir->write_mask;
   }

   for (int i = 0; i < 4; i++) {
  if (l.writemask & (1 << i)) {
 first_enabled_chan = GET_SWZ(r.swizzle, i);
 break;

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


Re: [Mesa-dev] [PATCH 2/2] st/glsl_to_tgsi: fix block copies of arrays of structs

2016-10-17 Thread Timothy Arceri
On Mon, 2016-10-17 at 19:25 +0200, Nicolai Hähnle wrote:
> From: Nicolai Hähnle 
> 
> Use a full writemask in this case. This is relevant e.g. when a
> function
> has an inout argument which is an array of structs.
> ---
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 1662f7f..b91ebaf 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -2964,24 +2964,26 @@ glsl_to_tgsi_visitor::visit(ir_assignment
> *ir)
>  
>   if (variable->data.location == FRAG_RESULT_DEPTH)
>  l.writemask = WRITEMASK_Z;
>   else {
>  assert(variable->data.location == FRAG_RESULT_STENCIL);
>  l.writemask = WRITEMASK_Y;
>   }
>    } else if (ir->write_mask == 0) {
>   assert(!ir->lhs->type->is_scalar() && !ir->lhs->type-
> >is_vector());
>  
> - if (ir->lhs->type->is_array() || ir->lhs->type-
> >is_matrix()) {
> -unsigned num_elements = ir->lhs->type->without_array()-
> >vector_elements;
> + unsigned num_elements = ir->lhs->type->without_array()-
> >vector_elements;
> +
> + if (num_elements) {
>  l.writemask = u_bit_consecutive(0, num_elements);
>   } else {
> +// The type is a struct or an array of (array of)
> structs.


C++ style comment in Mesa :(


>  l.writemask = WRITEMASK_XYZW;
>   }
>    } else {
>   l.writemask = ir->write_mask;
>    }
>  
>    for (int i = 0; i < 4; i++) {
>   if (l.writemask & (1 << i)) {
>  first_enabled_chan = GET_SWZ(r.swizzle, i);
>  break;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] st/glsl_to_tgsi: fix block copies of arrays of structs

2016-10-17 Thread Nicolai Hähnle
From: Nicolai Hähnle 

Use a full writemask in this case. This is relevant e.g. when a function
has an inout argument which is an array of structs.
---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 1662f7f..b91ebaf 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -2964,24 +2964,26 @@ glsl_to_tgsi_visitor::visit(ir_assignment *ir)
 
  if (variable->data.location == FRAG_RESULT_DEPTH)
 l.writemask = WRITEMASK_Z;
  else {
 assert(variable->data.location == FRAG_RESULT_STENCIL);
 l.writemask = WRITEMASK_Y;
  }
   } else if (ir->write_mask == 0) {
  assert(!ir->lhs->type->is_scalar() && !ir->lhs->type->is_vector());
 
- if (ir->lhs->type->is_array() || ir->lhs->type->is_matrix()) {
-unsigned num_elements = 
ir->lhs->type->without_array()->vector_elements;
+ unsigned num_elements = 
ir->lhs->type->without_array()->vector_elements;
+
+ if (num_elements) {
 l.writemask = u_bit_consecutive(0, num_elements);
  } else {
+// The type is a struct or an array of (array of) structs.
 l.writemask = WRITEMASK_XYZW;
  }
   } else {
  l.writemask = ir->write_mask;
   }
 
   for (int i = 0; i < 4; i++) {
  if (l.writemask & (1 << i)) {
 first_enabled_chan = GET_SWZ(r.swizzle, i);
 break;
-- 
2.7.4

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