<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, Oct 30, 2018 at 9:17 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">With the simplifications to this pass in a3b4cb34589e2f1a68 we<br>
can allow any alu instruction to be processed. For one this can<br>
potentially help with bcsels.<br></blockquote><div><br></div><div>Do we want to?  I believe that this patch is correct and I agree that we now can but then we get into some heuristics as to when it is or isn't useful.  With iand, ior, and inot it's obviously useful because, for boolean values, those instructions can be sort of half-folded, i.e. we know that opt_algebraic will look at them and replace them with either the other source or a constant.  With bcsel that's true if the condition is used in src[0] but not for src[1] or src[2].  For other instructions, it's even less obvious that it helps.</div><div><br></div><div>I'm not saying I'm opposed.  Just thinking out loud.<br></div><div><br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
No shader-db change.<br>
---<br>
 src/compiler/nir/nir_opt_if.c | 20 ++++----------------<br>
 1 file changed, 4 insertions(+), 16 deletions(-)<br>
<br>
diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c<br>
index 1fe95e53766..38489676e6b 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>
@@ -469,19 +469,6 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src,<br>
    return true;<br>
 }<br>
<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>
-<br>
-   return false;<br>
-}<br>
-<br>
 static bool<br>
 evaluate_condition_use(nir_builder *b, nir_if *nif, nir_src *use_src,<br>
                        bool is_if_condition)<br>
@@ -502,7 +489,8 @@ evaluate_condition_use(nir_builder *b, nir_if *nif, nir_src *use_src,<br>
       progress = true;<br>
    }<br>
<br>
-   if (!is_if_condition && can_propagate_through_alu(use_src)) {<br>
+   if (!is_if_condition &&<br>
+       use_src->parent_instr->type == nir_instr_type_alu) {<br>
       nir_alu_instr *alu = nir_instr_as_alu(use_src->parent_instr);<br>
<br>
       nir_foreach_use_safe(alu_use, &alu->dest.dest.ssa) {<br>
-- <br>
2.17.2<br>
<br>
</blockquote></div></div>