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

[Kenneth Graunke]

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.

In the past, we attempted to insert DEP_RESOLVE_MOVs to read every register
that was written by a SEND.  With your patch, we will only insert them for
components that were actually used.

I imagine this will work fine - if you read the G45 PRM Volume 4 page 516,
you'll see all kinds of dependency tracking woes.  Switching threads also
apparently solves the pre-send problem, so I imagine terminating the thread
without ever touching the register again would be safe.

I'm kind of tempted to keep the old behavior, out of paranoia, but I can
test it, at least.

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84613
> ---
> Untested on real hardware.
> 
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 9dfb7b7..3f9cd68 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -2925,16 +2925,6 @@ fs_visitor::insert_gen4_post_send_dependency_workarounds(bblock_t *block, fs_ins
>        if (i == write_len)
>           return;
>     }
> -
> -   /* If we hit the end of the program, resolve all remaining dependencies out
> -    * of paranoia.
> -    */
> -   fs_inst *last_inst = (fs_inst *)this->instructions.get_tail();
> -   assert(last_inst->eot);
> -   for (int i = 0; i < write_len; i++) {
> -      if (needs_dep[i])
> -         last_inst->insert_before(block, DEP_RESOLVE_MOV(first_write_grf + i));
> -   }
>  }
>  
>  void
-------------- 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/20150116/fc6fb282/attachment.sig>