[Mesa-dev] [PATCH 1/2] i965: Make emit_urb_writes() not produce an EOT message for GS.

[Kenneth Graunke]

On Monday, June 27, 2016 2:50:02 PM PDT Iago Toral wrote:
> On Sun, 2016-06-26 at 01:53 -0700, Kenneth Graunke wrote:
> > emit_urb_writes() contains code to emit an EOT write with no actual
> > data when there are no output varyings.  This makes sense for the VS
> > and TES stages, where it's called once at the end of the program.
> > 
> > However, in the geometry shader stage, emit_urb_writes() is called once
> > for every EmitVertex().  We explicitly emit a URB write with EOT set at
> > the end of the shader, separately from this path.  So we'd better not
> > terminate the thread.  This could get us into trouble for shaders which
> > do EmitVertex() with no varyings followed by SSBO/image/atomic writes.
> > 
> > It also caused us to emit multiple sends with EOT set, which apparently
> > confuses the register allocator into not using g112-g127 for all but
> > the first one.  This caused EU validation failures in OglGSCloth
> > shaders in shader-db.  (The actual application was fine, but shader-db
> > thinks there are no outputs because it doesn't understand transform
> > feedback.)
> > 
> > Cc: mesa-stable at lists.freedesktop.org
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> > index 3a49794..4a1ff30 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> > @@ -594,6 +594,10 @@ fs_visitor::emit_urb_writes(const fs_reg &gs_vertex_count)
> >      *    "The write data payload can be between 1 and 8 message phases long."
> >      */
> >     if (vue_map->slots_valid == 0) {
> > +      /* For GS, just turn EmitVertex() into a no-op. */
> 
> Maybe it would be better to explain in this comment why we can do this
> safely here, which as you say would be because for GS we will send a
> send with EOT set at the end of the shader in any case.
> 
> Both patches are:
> Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

Good call.  I've updated this locally to:

      /* For GS, just turn EmitVertex() into a no-op.  We don't want it to
       * end the thread, and emit_gs_thread_end() already emits a SEND with
       * EOT at the end of the program for us.
       */

Thanks Iago!
-------------- 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: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160627/e3252865/attachment.sig>