Mesa (master): nir: fix bug with moves in nir_opt_remove_phis()

Connor Abbott cwabbott0 at kemper.freedesktop.org
Sat Sep 3 04:40:17 UTC 2016


Module: Mesa
Branch: master
Commit: c62b58c2165a55c5e656efeafebc64da7acd8cca
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=c62b58c2165a55c5e656efeafebc64da7acd8cca

Author: Connor Abbott <cwabbott0 at gmail.com>
Date:   Fri Sep  2 19:07:57 2016 -0400

nir: fix bug with moves in nir_opt_remove_phis()

In 144cbf8 ("nir: Make nir_opt_remove_phis see through moves."), Ken
made nir_opt_remove_phis able to coalesce phi nodes whose sources are
all moves with the same swizzle. However, he didn't add the logic
necessary for handling the fact that the phi may now have multiple
different sources, even though the sources point to the same thing. For
example, if we had something like:

if (...)
   a1 = b.yx;
else
   a2 = b.yx;
a = phi(a1, a2)
... = a

then we would rewrite it to

if (...)
   a1 = b.yx;
else
   a2 = b.yx;
... = a1

by picking a random phi source, which in this case is invalid because
the source doesn't dominate the phi. Instead, we need to change it to:

if (...)
   a1 = b.yx;
else
   a2 = b.yx;
a3 = b.yx;
... = a3;

Fixes 12 CTS tests:
ES31-CTS.functional.tessellation.invariance.outer_edge_symmetry.quads*

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

---

 src/compiler/nir/nir_opt_remove_phis.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/compiler/nir/nir_opt_remove_phis.c b/src/compiler/nir/nir_opt_remove_phis.c
index ee92fbe..acaa6e1 100644
--- a/src/compiler/nir/nir_opt_remove_phis.c
+++ b/src/compiler/nir/nir_opt_remove_phis.c
@@ -26,6 +26,7 @@
  */
 
 #include "nir.h"
+#include "nir_builder.h"
 
 static nir_alu_instr *
 get_parent_mov(nir_ssa_def *ssa)
@@ -63,7 +64,7 @@ matching_mov(nir_alu_instr *mov1, nir_ssa_def *ssa)
  */
 
 static bool
-remove_phis_block(nir_block *block)
+remove_phis_block(nir_block *block, nir_builder *b)
 {
    bool progress = false;
 
@@ -113,6 +114,21 @@ remove_phis_block(nir_block *block)
        */
       assert(def != NULL);
 
+      if (mov) {
+         /* If the sources were all mov's from the same source with the same
+          * swizzle, then we can't just pick a random move because it may not
+          * dominate the phi node. Instead, we need to emit our own move after
+          * the phi which uses the shared source, and rewrite uses of the phi
+          * to use the move instead. This is ok, because while the mov's may
+          * not all dominate the phi node, their shared source does.
+          */
+
+         b->cursor = nir_after_phis(block);
+         def = mov->op == nir_op_imov ?
+            nir_imov_alu(b, mov->src[0], def->num_components) :
+            nir_fmov_alu(b, mov->src[0], def->num_components);
+      }
+
       assert(phi->dest.is_ssa);
       nir_ssa_def_rewrite_uses(&phi->dest.ssa, nir_src_for_ssa(def));
       nir_instr_remove(instr);
@@ -127,9 +143,11 @@ static bool
 remove_phis_impl(nir_function_impl *impl)
 {
    bool progress = false;
+   nir_builder bld;
+   nir_builder_init(&bld, impl);
 
    nir_foreach_block(block, impl) {
-      progress |= remove_phis_block(block);
+      progress |= remove_phis_block(block, &bld);
    }
 
    if (progress) {




More information about the mesa-commit mailing list