On Thu, 2016-12-22 at 11:42 -0800, Jason Ekstrand wrote: > On Thu, Dec 22, 2016 at 3:57 AM, Timothy Arceri <timothy.arceri@colla > bora.com> wrote: > > On Wed, 2016-12-21 at 21:11 -0800, Jason Ekstrand wrote: > > > On Dec 21, 2016 9:17 PM, "Timothy Arceri" <timothy.arceri@collabo > > ra.c > > > om> wrote: > > > On Mon, 2016-12-19 at 20:11 -0800, Jason Ekstrand wrote: > > > > --- > > > > src/mesa/drivers/dri/i965/brw_nir.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/src/mesa/drivers/dri/i965/brw_nir.c > > > > b/src/mesa/drivers/dri/i965/brw_nir.c > > > > index 0c1fb44..a091861 100644 > > > > --- a/src/mesa/drivers/dri/i965/brw_nir.c > > > > +++ b/src/mesa/drivers/dri/i965/brw_nir.c > > > > @@ -429,6 +429,8 @@ nir_optimize(nir_shader *nir, const struct > > > > brw_compiler *compiler, > > > > OPT(nir_opt_algebraic); > > > > OPT(nir_opt_constant_folding); > > > > OPT(nir_opt_dead_cf); > > > > + OPT(nir_opt_trivial_continues); > > > > + OPT(nir_opt_if); > > > > > > I'm seeing regressions in my series for Vulkan (the Vulkan CTS > > was > > > only > > > enabled for my branch today). I wonder if we should be applying > > your > > > series before mine? Either way this or a similar patch is: > > > > > > If you're seeing regressions, there's probably a real bug there. > > I > > > assume you applied the else case fix? > > > > Yes its applied. I haven't checked them all but it seems at least > > some > > are falling over because of a redundant continue at the end of the > > loop. So to me it seems we should probably at least land > > nir_opt_trivial_continues before my series. Loop analysis will bail > > out > > if we detect a continue in an if branch but we expect no jumps in > > the > > loop body itself. > > > > I did a little digging and came to that same conclusion. I've got a > patch that fixes it (and, IMHO, cleans a few things up) on my > jenkins_vulkan branch. We'll see how the CTS likes it but it looks > good when I run dEQP-VK.glsl.loops.* on my laptop. > > While I was at it, I realized that we don't yet handle the fairly > stupid case of > > while (true) { > // Do Stuff > break; > } > > I'm not sure if it's our job or the job of dead_cf to handle that, > but it should get handled somewhere. That smells like a follow-on > patch to me.
I was sure we handled by deaf cf this but looking at again it doesn't look like we do. Should be easy enough to take care of somewhere. > > --Jason > > > > > > > Reviewed-by: Timothy Arceri <timothy.arc...@collabora.com> > > > > > > > if (nir->options->max_unroll_iterations != 0) { > > > > OPT(nir_opt_loop_unroll, indirect_mask); > > > > } > > > > > > _______________________________________________ > > > mesa-dev mailing list > > > mesa-dev@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev