i965/skl: Add the header for constant loads outside of the generator
authorNeil Roberts <neil@linux.intel.com>
Tue, 24 Mar 2015 15:52:20 +0000 (15:52 +0000)
committerNeil Roberts <neil@linux.intel.com>
Thu, 16 Apr 2015 12:02:26 +0000 (13:02 +0100)
commit33f73e93ff6e14f72153d3df7e80763137fcb943
treec87e9ed952d564c39d8bbc3ee12dc0a3d9d4243a
parenta9e4cf5d323dbf11e42deda389ed03db571a7df7
i965/skl: Add the header for constant loads outside of the generator

Commit 5a06ee738 added a step to the generator to set up the message
header when generating the VS_OPCODE_PULL_CONSTANT_LOAD_GEN7
instruction. That pseudo opcode is implemented in terms of multiple
actual opcodes, one of which writes to one of the source registers in
order to set up the message header. This causes problems because the
scheduler isn't aware that the source register is written to and it
can end up reorganising the instructions incorrectly such that the
write to the source register overwrites a needed value from a previous
instruction. This problem was presenting itself as a rendering error
in the weapon in Enemy Territory: Quake Wars.

Since commit 588859e1 there is an additional problem that the double
register allocated to include the message header would end up being
split into two. This wasn't happening previously because the code to
split registers was explicitly avoided for instructions that are
sending from the GRF.

This patch fixes both problems by splitting the code to set up the
message header into a new pseudo opcode so that it will be done
outside of the generator. This new opcode has the header register as a
destination so the scheduler can recognise that the register is
written to. This has the additional benefit that the scheduler can
optimise the message header slightly better by moving the mov
instructions further away from the send instructions.

On Skylake it appears to fix the following three Piglit tests without
causing any regressions:

 gs-float-array-variable-index
 gs-mat3x4-row-major
 gs-mat4x3-row-major

I think we actually may need to do something similar for the fs
backend and possibly for message headers from regular texture sampling
but I'm not entirely sure.

v2: Make sure the exec-size is retained as 8 for the mov instruction
    to initialise the header from g0. This was accidentally lost
    during a rebase on top of 07c571a39fa1.
    Split the patch into two so that the helper function is a separate
    change.
    Fix emitting the MOV instruction on Gen7.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89058
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
src/mesa/drivers/dri/i965/brw_defines.h
src/mesa/drivers/dri/i965/brw_shader.cpp
src/mesa/drivers/dri/i965/brw_vec4.h
src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp
src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp