[Mesa-dev] [PATCH 1/2] nir/dead_cf: Don't crash on unreachable after-loop blocks

[Kenneth Graunke]

On Thursday, September 1, 2016 7:00:01 PM PDT Connor Abbott wrote:
> On Thu, Sep 1, 2016 at 6:19 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> > On Thu, Sep 1, 2016 at 12:41 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
> >>
> >> As-is, this change will make us delete trivially infinite loops (i.e.
> >> loops with no break statement). The most likely scenario where we get
> >> one of those (besides for silly Piglit tests) is a developer with an
> >> accidental bug in their shader. In that case, it seems kinda mean to
> >> then delete the entire loop and pretend like it doesn't exist. Maybe
> >> add something like
> >>
> >> if (!after->imm_dom)
> >>    return false;
> >>
> >> with a comment explaining what's going on? This would also help us
> >> catch other places where we don't handle infinite loops correctly that
> >> might've been hidden by this.
> >
> >
> > I wrote that patch and kicked it off to Jenkins and it passed just fine.
> > However, I don't know if I like it.  Matt and I were talking about this
> > yesterday and his suggestion was to, if they have an infinite loop, delete
> > the entire shader and draw pink.  That's more useful to developers than
> > hanging their GPU.
> 
> Hmm... I guess if you did that it would be wise to also print a
> message somewhere the developer could see, since "uhh now the entire
> thing is pink" probably won't help clarify what the problem is, even
> if it shows that there is in fact a problem :)
> 
> But I guess Matt is right that the danger is more theoretical than
> practical. So go ahead and add my r-b to both patches with or without
> the suggestion. Fake edges begone!

It would be great to be able to emit a warning to the shader info log,
or a KHR debug message.  We've never hooked that up to NIR, but if we
did, we could issue a warning and then basically do whatever we want
with impunity.

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160902/69af12de/attachment.sig>