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

[Iago Toral]

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>

> +      if (stage == MESA_SHADER_GEOMETRY)
> +         return;
> +
>        fs_reg payload = fs_reg(VGRF, alloc.allocate(2), BRW_REGISTER_TYPE_UD);
>        bld.exec_all().MOV(payload, urb_handle);
>