On Thu, Dec 22, 2016 at 3:57 AM, Timothy Arceri < timothy.arc...@collabora.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@collabora.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. --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