[Mesa-dev] [PATCH] nir/remove_phis: handle trivial back-edges

[Jason Ekstrand]

This looks totally sane to me.

Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

I'll try and get some shader-db numbers for you tomorrow.
--Jason

On May 21, 2015 5:07 PM, "Connor Abbott" <cwabbott0 at gmail.com> wrote:
>
> Some loops may have phi nodes that look like:
>
> foo = ...
> loop {
>     bar = phi(foo, bar)
>     ...
> }
>
> in which case we can remove the phi node and replace all uses of 'bar'
> with 'foo'. In particular, there are some L4D2 vertex shaders with loops
> that, after optimization, look like:
>
>         /* succs: block_1 */
>         loop {
>                 block block_1:
>                 /* preds: block_0 block_4 */
>                 vec1 ssa_2195 = phi block_0: ssa_2136, block_4: ssa_994
>                 vec1 ssa_7321 = phi block_0: ssa_8195, block_4: ssa_7321
>                 vec1 ssa_7324 = phi block_0: ssa_8198, block_4: ssa_7324
>                 vec1 ssa_7327 = phi block_0: ssa_8174, block_4: ssa_7327
>                 vec1 ssa_8139 = intrinsic load_uniform () () (232)
>                 vec1 ssa_588 = ige ssa_2195, ssa_8139
>                 /* succs: block_2 block_3 */
>                 if ssa_588 {
>                         block block_2:
>                         /* preds: block_1 */
>                         break
>                         /* succs: block_5 */
>                 } else {
>                         block block_3:
>                         /* preds: block_1 */
>                         /* succs: block_4 */
>                 }
>                 block block_4:
>                 /* preds: block_3 */
>                 vec1 ssa_994 = iadd ssa_2195, ssa_2150
>                 /* succs: block_1 */
>         }
>
> where after removing the second, third, and fourth phi nodes, the loop
> becomes entirely dead, and this patch combined with my nir-dead-cf-v4
> branch will cause the loop to be deleted entirely.
>
> No piglit regressions.
>
> Signed-off-by: Connor Abbott <cwabbott0 at gmail.com>
> ---
>  src/glsl/nir/nir_opt_remove_phis.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/src/glsl/nir/nir_opt_remove_phis.c
b/src/glsl/nir/nir_opt_remove_phis.c
> index 7896584..3660413 100644
> --- a/src/glsl/nir/nir_opt_remove_phis.c
> +++ b/src/glsl/nir/nir_opt_remove_phis.c
> @@ -60,6 +60,21 @@ remove_phis_block(nir_block *block, void *state)
>
>        nir_foreach_phi_src(phi, src) {
>           assert(src->src.is_ssa);
> +
> +         /* For phi nodes at the beginning of loops, we may encounter
some
> +          * sources from backedges that point back to the destination of
the
> +          * same phi, i.e. something like:
> +          *
> +          * a = phi(a, b, ...)
> +          *
> +          * We can safely ignore these sources, since if all of the
normal
> +          * sources point to the same definition, then that definition
must
> +          * still dominate the phi node, and the phi will still always
take
> +          * the value of that definition.
> +          */
> +
> +         if (src->src.ssa == &phi->dest.ssa)
> +            continue;
>
>           if (def == NULL) {
>              def  = src->src.ssa;
> @@ -74,6 +89,11 @@ remove_phis_block(nir_block *block, void *state)
>        if (!srcs_same)
>           continue;
>
> +      /* We must have found at least one definition, since there must be
at
> +       * least one forward edge.
> +       */
> +      assert(def != NULL);
> +
>        assert(phi->dest.is_ssa);
>        nir_ssa_def_rewrite_uses(&phi->dest.ssa, nir_src_for_ssa(def),
>                                 mem_ctx);
> --
> 2.1.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150521/a2c8d590/attachment.html>