Re: [Mesa-dev] [PATCH] nir: allow propagation of if evaluation for bcsel

2018-11-02 Thread Samuel Pitoiset 

This change breaks:

dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_frag,Fail
dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_geom,Fail
dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_tessc,Fail
dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_vert,Fail

On 11/1/18 10:53 PM, Timothy Arceri wrote:

Cc: Jason Ekstrand 
---
  src/compiler/nir/nir_opt_if.c | 26 +-
  1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index 1fe95e53766..50e8947eaa1 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -448,7 +448,7 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, 
nir_src *use_src,
 if (!evaluate_if_condition(nif, b->cursor, &bool_value))
return false;
  
-   nir_ssa_def *def[2] = {0};

+   nir_ssa_def *def[4] = {0};
 for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {
if (alu->src[i].src.ssa == use_src->ssa) {
   def[i] = nir_imm_bool(b, bool_value);
@@ -456,7 +456,7 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, 
nir_src *use_src,
   def[i] = alu->src[i].src.ssa;
}
 }
-   nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
+   nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], def[2], 
def[3]);
  
 /* Rewrite use to use new alu instruction */

 nir_src new_src = nir_src_for_ssa(nalu);
@@ -472,14 +472,22 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, 
nir_src *use_src,
  static bool
  can_propagate_through_alu(nir_src *src)
  {
-   if (src->parent_instr->type == nir_instr_type_alu &&
-   (nir_instr_as_alu(src->parent_instr)->op == nir_op_ior ||
-nir_instr_as_alu(src->parent_instr)->op == nir_op_iand ||
-nir_instr_as_alu(src->parent_instr)->op == nir_op_inot ||
-nir_instr_as_alu(src->parent_instr)->op == nir_op_b2i))
-  return true;
+   if (src->parent_instr->type != nir_instr_type_alu)
+  return false;
  
-   return false;

+   switch (nir_instr_as_alu(src->parent_instr)->op) {
+  case nir_op_ior:
+  case nir_op_iand:
+  case nir_op_inot:
+  case nir_op_b2i:
+ return true;
+  case nir_op_bcsel:
+ if (src == &nir_instr_as_alu(src->parent_instr)->src[0].src)
+return true;
+ /* fall through */
+  default:
+ return false;
+   }
  }
  
  static bool



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

Re: [Mesa-dev] [PATCH] nir: allow propagation of if evaluation for bcsel

2018-11-01 Thread Jason Ekstrand 
On Thu, Nov 1, 2018 at 4:53 PM Timothy Arceri  wrote:

> Cc: Jason Ekstrand 
> ---
>  src/compiler/nir/nir_opt_if.c | 26 +-
>  1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
> index 1fe95e53766..50e8947eaa1 100644
> --- a/src/compiler/nir/nir_opt_if.c
> +++ b/src/compiler/nir/nir_opt_if.c
> @@ -448,7 +448,7 @@ propagate_condition_eval(nir_builder *b, nir_if *nif,
> nir_src *use_src,
> if (!evaluate_if_condition(nif, b->cursor, &bool_value))
>return false;
>
> -   nir_ssa_def *def[2] = {0};
> +   nir_ssa_def *def[4] = {0};
> for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {
>if (alu->src[i].src.ssa == use_src->ssa) {
>   def[i] = nir_imm_bool(b, bool_value);
> @@ -456,7 +456,7 @@ propagate_condition_eval(nir_builder *b, nir_if *nif,
> nir_src *use_src,
>   def[i] = alu->src[i].src.ssa;
>}
> }
> -   nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], NULL,
> NULL);
> +   nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], def[2],
> def[3]);
>
> /* Rewrite use to use new alu instruction */
> nir_src new_src = nir_src_for_ssa(nalu);
> @@ -472,14 +472,22 @@ propagate_condition_eval(nir_builder *b, nir_if
> *nif, nir_src *use_src,
>  static bool
>  can_propagate_through_alu(nir_src *src)
>  {
> -   if (src->parent_instr->type == nir_instr_type_alu &&
> -   (nir_instr_as_alu(src->parent_instr)->op == nir_op_ior ||
> -nir_instr_as_alu(src->parent_instr)->op == nir_op_iand ||
> -nir_instr_as_alu(src->parent_instr)->op == nir_op_inot ||
> -nir_instr_as_alu(src->parent_instr)->op == nir_op_b2i))
> -  return true;
> +   if (src->parent_instr->type != nir_instr_type_alu)
> +  return false;
>

We can do "nir_alu_instr *alu = nir_instr_as_alu(src->parent_instr)" here
and simplify the code below.


> -   return false;
> +   switch (nir_instr_as_alu(src->parent_instr)->op) {
> +  case nir_op_ior:
> +  case nir_op_iand:
> +  case nir_op_inot:
> +  case nir_op_b2i:
> + return true;
> +  case nir_op_bcsel:
> + if (src == &nir_instr_as_alu(src->parent_instr)->src[0].src)
> +return true;
> + /* fall through */
>

There's no good reason for the fall-through.  Just return false here.  Or,
better yet, "return src == &alu->src[0].src"

With those changes,

Reviewed-by: Jason Ekstrand 


> +  default:
> + return false;
> +   }
>  }
>
>  static bool
> --
> 2.19.1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] nir: allow propagation of if evaluation for bcsel

2018-11-01 Thread Timothy Arceri 
Cc: Jason Ekstrand 
---
 src/compiler/nir/nir_opt_if.c | 26 +-
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index 1fe95e53766..50e8947eaa1 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -448,7 +448,7 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, 
nir_src *use_src,
if (!evaluate_if_condition(nif, b->cursor, &bool_value))
   return false;
 
-   nir_ssa_def *def[2] = {0};
+   nir_ssa_def *def[4] = {0};
for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {
   if (alu->src[i].src.ssa == use_src->ssa) {
  def[i] = nir_imm_bool(b, bool_value);
@@ -456,7 +456,7 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, 
nir_src *use_src,
  def[i] = alu->src[i].src.ssa;
   }
}
-   nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
+   nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], def[2], 
def[3]);
 
/* Rewrite use to use new alu instruction */
nir_src new_src = nir_src_for_ssa(nalu);
@@ -472,14 +472,22 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, 
nir_src *use_src,
 static bool
 can_propagate_through_alu(nir_src *src)
 {
-   if (src->parent_instr->type == nir_instr_type_alu &&
-   (nir_instr_as_alu(src->parent_instr)->op == nir_op_ior ||
-nir_instr_as_alu(src->parent_instr)->op == nir_op_iand ||
-nir_instr_as_alu(src->parent_instr)->op == nir_op_inot ||
-nir_instr_as_alu(src->parent_instr)->op == nir_op_b2i))
-  return true;
+   if (src->parent_instr->type != nir_instr_type_alu)
+  return false;
 
-   return false;
+   switch (nir_instr_as_alu(src->parent_instr)->op) {
+  case nir_op_ior:
+  case nir_op_iand:
+  case nir_op_inot:
+  case nir_op_b2i:
+ return true;
+  case nir_op_bcsel:
+ if (src == &nir_instr_as_alu(src->parent_instr)->src[0].src)
+return true;
+ /* fall through */
+  default:
+ return false;
+   }
 }
 
 static bool
-- 
2.19.1

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