<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Dec 22, 2016 at 3:57 AM, Timothy Arceri <span dir="ltr"><<a href="mailto:timothy.arceri@collabora.com" target="_blank">timothy.arceri@collabora.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, 2016-12-21 at 21:11 -0800, Jason Ekstrand wrote:<br>
> On Dec 21, 2016 9:17 PM, "Timothy Arceri" <timothy.arceri@collabora.c<br>
> om> wrote:<br>
> On Mon, 2016-12-19 at 20:11 -0800, Jason Ekstrand wrote:<br>
> > ---<br>
> >  src/mesa/drivers/dri/i965/<wbr>brw_nir.c | 2 ++<br>
> >  1 file changed, 2 insertions(+)<br>
> ><br>
> > diff --git a/src/mesa/drivers/dri/i965/<wbr>brw_nir.c<br>
> > b/src/mesa/drivers/dri/i965/<wbr>brw_nir.c<br>
> > index 0c1fb44..a091861 100644<br>
> > --- a/src/mesa/drivers/dri/i965/<wbr>brw_nir.c<br>
> > +++ b/src/mesa/drivers/dri/i965/<wbr>brw_nir.c<br>
> > @@ -429,6 +429,8 @@ nir_optimize(nir_shader *nir, const struct<br>
> > brw_compiler *compiler,<br>
> >        OPT(nir_opt_algebraic);<br>
> >        OPT(nir_opt_constant_<wbr>folding);<br>
> >        OPT(nir_opt_dead_cf);<br>
> > +      OPT(nir_opt_trivial_<wbr>continues);<br>
> > +      OPT(nir_opt_if);<br>
><br>
> I'm seeing regressions in my series for Vulkan (the Vulkan CTS was<br>
> only<br>
> enabled for my branch today).  I wonder if we should be applying your<br>
> series before mine? Either way this or a similar patch is:<br>
><br>
> If you're seeing regressions, there's probably a real bug there.  I<br>
> assume you applied the else case fix?<br>
<br>
</div></div>Yes its applied. I haven't checked them all but it seems at least some<br>
are falling over because of a redundant continue at the end of the<br>
loop. So to me it seems we should probably at least land<br>
nir_opt_trivial_continues before my series. Loop analysis will bail out<br>
if we detect a continue in an if branch but we expect no jumps in the<br>
loop body itself.<span class=""><br></span></blockquote><div><br></div><div>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.<br><br></div><div>While I was at it, I realized that we don't yet handle the fairly stupid case of<br><br></div><div>while (true) {<br></div><div>   // Do Stuff<br></div><div>   break;<br>}<br><br></div><div>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.<br><br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
><br>
> Reviewed-by: Timothy Arceri <<a href="mailto:timothy.arceri@collabora.com">timothy.arceri@collabora.com</a>><br>
><br>
> >        if (nir->options->max_unroll_<wbr>iterations != 0) {<br>
> >           OPT(nir_opt_loop_<wbr>unroll, indirect_mask);<br>
> >        }<br>
><br>
</span>> ______________________________<wbr>_________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</blockquote></div><br></div></div>