<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Nov 1, 2018 at 4:53 PM Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com">tarceri@itsqueeze.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Cc: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>><br>
---<br>
 src/compiler/nir/nir_opt_if.c | 26 +++++++++++++++++---------<br>
 1 file changed, 17 insertions(+), 9 deletions(-)<br>
<br>
diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c<br>
index 1fe95e53766..50e8947eaa1 100644<br>
--- a/src/compiler/nir/nir_opt_if.c<br>
+++ b/src/compiler/nir/nir_opt_if.c<br>
@@ -448,7 +448,7 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src,<br>
    if (!evaluate_if_condition(nif, b->cursor, &bool_value))<br>
       return false;<br>
<br>
-   nir_ssa_def *def[2] = {0};<br>
+   nir_ssa_def *def[4] = {0};<br>
    for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {<br>
       if (alu->src[i].src.ssa == use_src->ssa) {<br>
          def[i] = nir_imm_bool(b, bool_value);<br>
@@ -456,7 +456,7 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src,<br>
          def[i] = alu->src[i].src.ssa;<br>
       }<br>
    }<br>
-   nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);<br>
+   nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], def[2], def[3]);<br>
<br>
    /* Rewrite use to use new alu instruction */<br>
    nir_src new_src = nir_src_for_ssa(nalu);<br>
@@ -472,14 +472,22 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src,<br>
 static bool<br>
 can_propagate_through_alu(nir_src *src)<br>
 {<br>
-   if (src->parent_instr->type == nir_instr_type_alu &&<br>
-       (nir_instr_as_alu(src->parent_instr)->op == nir_op_ior ||<br>
-        nir_instr_as_alu(src->parent_instr)->op == nir_op_iand ||<br>
-        nir_instr_as_alu(src->parent_instr)->op == nir_op_inot ||<br>
-        nir_instr_as_alu(src->parent_instr)->op == nir_op_b2i))<br>
-      return true;<br>
+   if (src->parent_instr->type != nir_instr_type_alu)<br>
+      return false;<br></blockquote><div><br></div><div>We can do "nir_alu_instr *alu = nir_instr_as_alu(src->parent_instr)" here and simplify the code below.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
-   return false;<br>
+   switch (nir_instr_as_alu(src->parent_instr)->op) {<br>
+      case nir_op_ior:<br>
+      case nir_op_iand:<br>
+      case nir_op_inot:<br>
+      case nir_op_b2i:<br>
+         return true;<br>
+      case nir_op_bcsel:<br>
+         if (src == &nir_instr_as_alu(src->parent_instr)->src[0].src)<br>
+            return true;<br>
+         /* fall through */<br></blockquote><div><br></div><div>There's no good reason for the fall-through.  Just return false here.  Or, better yet, "return src == &alu->src[0].src"</div><div><br></div><div>With those changes,</div><div><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+      default:<br>
+         return false;<br>
+   }<br>
 }<br>
<br>
 static bool<br>
-- <br>
2.19.1<br>
<br>
</blockquote></div></div>