i965 gs: Clean up dodgy register re-use, at the cost of a few MOVs.
authorPaul Berry <stereotype441@gmail.com>
Tue, 29 Nov 2011 22:54:02 +0000 (14:54 -0800)
committerPaul Berry <stereotype441@gmail.com>
Thu, 8 Dec 2011 00:38:01 +0000 (16:38 -0800)
commit3f2283172bcaf3db00a99baad0319bc7e0be5fc2
tree17d03e1c1a7be0be83bf9f46961456d6cd9ce974
parent43e39b58c705714c01919e5b4b5566e82e803d58
i965 gs: Clean up dodgy register re-use, at the cost of a few MOVs.

Prior to this patch, in the Gen4 and Gen5 GS, we used GRF 0 (called
"R0" in the code) as a staging area to prepare the message header for
the FF_SYNC and URB_WRITE messages.  This cleverly avoided an
unnecessary MOV operation (since the initial value of GRF 0 contains
data that needs to be included in the message header), but it made the
code confusing, since GRF 0 could no longer be relied upon to contain
its initial value once the GS started preparing its first message.
This patch avoids confusion by using a separate register ("header") as
the staging area, at the cost of one MOV instruction.

Worse yet, prior to this patch, the GS would completely overwrite the
contents of GRF 0 with the writeback data it received from a completed
FF_SYNC or URB_WRITE message.  It did this because DWORD 0 of the
writeback data contains the new URB handle, and that neds to be
included in DWORD 0 of the next URB_WRITE message header.  However,
that caused the rest of the message header to be corrupted either with
undefined data or zeros.  Astonishingly, this did not produce any
known failures (probably by dumb luck).  However, it seems really
dodgy--corrupting FFTID in particular seems likely to cause GPU hangs.
This patch avoids the corruption by storing the writeback data in a
temporary register and then copying just DWORD 0 to the header for the
next message.  This costs one extra MOV instruction per message sent,
except for the final message.

Also, this patch moves the logic for overriding DWORD 2 of the header
(which contains PrimType, PrimStart, PrimEnd, and some other data that
we don't care about yet).  This logic is now in the function
brw_gs_overwrite_header_dw2() rather than in brw_gs_emit_vue().  This
saves one MOV instruction in brw_gs_quads() and brw_gs_quad_strip(),
and paves the way for the Gen6 GS, which will need more complex logic
to override DWORD 2 of the header.

Finally, the function brw_gs_alloc_regs() contained a benign bug: it
neglected to increment the register counter when allocating space for
the "temp" register.  This turned out not to have any effect because
the temp register wasn't used on Gen4 and Gen5, the only hardware
models (so far) to require a GS program.  Now, all the registers
allocated by brw_gs_alloc_regs() are actually used, and properly
accounted for.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
src/mesa/drivers/dri/i965/brw_gs.h
src/mesa/drivers/dri/i965/brw_gs_emit.c