[Mesa-dev] [PATCH] Revert "nir: Make nir_opt_remove_phis see through moves."

[Connor Abbott]

I think we can just fix this by emitting another move with the right
swizzle. Then DCE should clean up the original moves, so it should
still be a win. Let me send a patch.

On Fri, Sep 2, 2016 at 4:50 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> This reverts commit 144cbf89879103c45f79b7c5b923ebad63f08c55.
>
> It doesn't work.  Given a phi after if/else blocks where both phi
> sources are identical MOVs, it rewrites the phi destination to use
> the first def it encounters...namely the result of the mov in the
> then block.  This results in the else branch's value being thrown
> out entirely, which is quite bogus.
>
> Fixes 12 CTS tests:
> ES31-CTS.functional.tessellation.invariance.outer_edge_symmetry.quads*
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/compiler/nir/nir_opt_remove_phis.c | 24 +-----------------------
>  1 file changed, 1 insertion(+), 23 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_remove_phis.c b/src/compiler/nir/nir_opt_remove_phis.c
> index ee92fbe..f1e7bb0 100644
> --- a/src/compiler/nir/nir_opt_remove_phis.c
> +++ b/src/compiler/nir/nir_opt_remove_phis.c
> @@ -27,26 +27,6 @@
>
>  #include "nir.h"
>
> -static nir_alu_instr *
> -get_parent_mov(nir_ssa_def *ssa)
> -{
> -   if (ssa->parent_instr->type != nir_instr_type_alu)
> -      return NULL;
> -
> -   nir_alu_instr *alu = nir_instr_as_alu(ssa->parent_instr);
> -   return (alu->op == nir_op_imov || alu->op == nir_op_fmov) ? alu : NULL;
> -}
> -
> -static bool
> -matching_mov(nir_alu_instr *mov1, nir_ssa_def *ssa)
> -{
> -   if (!mov1)
> -      return false;
> -
> -   nir_alu_instr *mov2 = get_parent_mov(ssa);
> -   return mov2 && nir_alu_srcs_equal(mov1, mov2, 0, 0);
> -}
> -
>  /*
>   * This is a pass for removing phi nodes that look like:
>   * a = phi(b, b, b, ...)
> @@ -74,7 +54,6 @@ remove_phis_block(nir_block *block)
>        nir_phi_instr *phi = nir_instr_as_phi(instr);
>
>        nir_ssa_def *def = NULL;
> -      nir_alu_instr *mov = NULL;
>        bool srcs_same = true;
>
>        nir_foreach_phi_src(src, phi) {
> @@ -96,9 +75,8 @@ remove_phis_block(nir_block *block)
>
>           if (def == NULL) {
>              def  = src->src.ssa;
> -            mov = get_parent_mov(def);
>           } else {
> -            if (src->src.ssa != def && !matching_mov(mov, src->src.ssa)) {
> +            if (src->src.ssa != def) {
>                 srcs_same = false;
>                 break;
>              }
> --
> 2.9.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev