This looks totally sane to me. Reviewed-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> I'll try and get some shader-db numbers for you tomorrow. --Jason On May 21, 2015 5:07 PM, "Connor Abbott" <<a href="mailto:cwabbott0@gmail.com">cwabbott0@gmail.com</a>> 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 <<a href="mailto:cwabbott0@gmail.com">cwabbott0@gmail.com</a>> > --- > 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 > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>