It seems I initially wrote this as:

if (cond_for_opt) {
  <opt code>

and then I modified the style at some point to be like:

if (!cond_for_opt)
<opt code>

But I did not re-write all the conditions accordingly.

I tested this quickly on a haswell with our fp64 branch and it did not
show any regressions. I verified the optimization was being triggered
(it kicked in for 210 out of 1303 shader tests, which includes vec4 tests,
so it kicks in a lot).

I also noticed that at least in a couple of cases that I inspected manually,
copy propagation was able to achieve the same result, making this opt
unnecessary, but that might not always work out, since removing the same
optimization for the unpack from previous pack case led to worse code in the
cases that I inspected manually, so I guess we should keep both optimizations
for now.

 src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
index 4e68ffb..6307b5c 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -1208,16 +1208,16 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, 
nir_alu_instr *instr)
        * the unpack operation.
       for (int i = 0; i < 2; i++) {
-         if (instr->src[i].src.is_ssa)
+         if (!instr->src[i].src.is_ssa)
          const nir_instr *parent_instr = instr->src[i].src.ssa->parent_instr;
-         if (parent_instr->type == nir_instr_type_alu)
+         if (parent_instr->type != nir_instr_type_alu)
          const nir_alu_instr *alu_parent = nir_instr_as_alu(parent_instr);
-         if (alu_parent->op == nir_op_unpack_double_2x32_split_x ||
-             alu_parent->op == nir_op_unpack_double_2x32_split_y)
+         if (alu_parent->op != nir_op_unpack_double_2x32_split_x &&
+             alu_parent->op != nir_op_unpack_double_2x32_split_y)
          if (!alu_parent->src[0].src.is_ssa)

mesa-dev mailing list

Reply via email to