i965/skl: Add the header for constant loads outside of the generator

Graphics / Mesa 3D Graphics Library / Mesa - Neil Roberts [linux.intel.com] - 16 April 2015 07:02 UTC

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

33f73e9 i965/skl: Add the header for constant loads outside of the generator
src/mesa/drivers/dri/i965/brw_defines.h | 1 +
src/mesa/drivers/dri/i965/brw_shader.cpp | 4 ++
src/mesa/drivers/dri/i965/brw_vec4.h | 2 +
.../dri/i965/brw_vec4_dead_code_eliminate.cpp | 1 +
src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 52 ++++++++++----------
src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 38 ++++++++++----
6 files changed, 63 insertions(+), 35 deletions(-)

Upstream: cgit.freedesktop.org

  • Share