[Mesa-dev] [PATCH 1/2] i965/fs: Don't use backend_visitor::instructions after creating the CFG.

[Kenneth Graunke]

On Friday, January 16, 2015 11:55:33 PM Matt Turner wrote:
> On Fri, Jan 16, 2015 at 11:45 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> > On Tuesday, January 13, 2015 03:35:57 PM Matt Turner wrote:
> >> This is a fix for a regression introduced in commit a9f8296d ("i965/fs:
> >> Preserve the CFG in a few more places.").
> >>
> >> The errata this code works around is described in a comment before the function:
> >>
> >>    "[DevBW, DevCL] Errata: A destination register from a send can not be
> >>     used as a destination register until after it has been sourced by an
> >>     instruction with a different destination register.
> >>
> >> The framebuffer write's sources must be in message registers, which SEND
> >> instructions cannot have as a destination. There's no way for this
> >> errata to affect anything at the end of the program. Just remove the
> >> code.
> >
> > I don't think that's the point.  The idea is that code such as
> >
> >    SEND g10  ...sources... rlen 4
> >    MUL  g10  ... ...
> >
> > needs a workaround - you can't write to the destination of a SEND safely
> > without reading them first.  You'd have to do:
> >
> >    SEND g10  ...sources... rlen 4
> >    MOV  null g10           pointless read of g10, any instruction will do
> >    MUL  g10  ...
> >
> > Normally, the results of SEND instructions are actually used.  However, they
> > aren't always - for example, depth texturing returns 4 values, but we only
> > care about the .X channel.
> 
> Right, and we throw up our hands and resolve all remaining
> dependencies when we see the end of the basic block because there's a
> subsequent basic block that may write the destination.
> 
> At the end of the program though... we can't possibly need to resolve
> anything outstanding because we can't possibly overwrite it. Can we?

I agree, I think this should be safe.  It sounds like the effects of the bug
are an undefined write ordering...probably not GPU hangs.  If that's true,
then we're obviously fine - we never overwrite it.

On the completely paranoid side of things, there could be some bit in the
hardware that leaves the register stuck: "I'm not done with the last write,
I need to stall until it completes before doing this one".  And, it's
possible it could persist between threads.  Which would leave us stalled
forever, and we'd hang the GPU.

But I sincerely doubt that's the case, and I agree with you that this should
be fine.  I would like to see the commit message updated - instead of the bit
about MRFs, say that we think it's pointless to apply the workaround for
registers that are never written again, and that deleting the code is an
alternative to making it work in CFG-land.

With an updated commit message and Piglit passing (I'll test and let you know),
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150117/5903d5c8/attachment.sig>