[Mesa-dev] [PATCH v3] i965/fs/gen7: split instructions that run into exec masking bugs

[Iago Toral Quiroga]

In fp64 we can produce code like this:

mov(16) vgrf2<2>:UD, vgrf3<2>:UD

That our simd lowering pass would typically split in instructions with a
width of 8, writing to two consecutive registers each. Unfortunately, gen7
hardware has a bug affecting execution masking and as a result, the
second GRF register write won't work properly. Curro verified this:

"The problem is that pre-Gen8 EUs are hardwired to use the QtrCtrl+1
 (where QtrCtrl is the 8-bit quarter of the execution mask signals
 specified in the instruction control fields) for the second
 compressed half of any single-precision instruction (for
 double-precision instructions it's hardwired to use NibCtrl+1,
 at least on HSW), which means that the EU will apply the wrong
 execution controls for the second sequential GRF write if the number
 of channels per GRF is not exactly eight in single-precision mode (or
 four in double-float mode)."

In practice, this means that we cannot write more than one
consecutive GRF in a single instruction if the number of channels
per GRF is not exactly eight in single-precision mode (or four
in double-float mode).

This patch makes our SIMD lowering pass split this kind of instructions
so that the split versions only write to a single register. In the
example above this means that we split the write in 4 instructions, each
one writing 4 UD elements (width = 4) to a single register.

v2 (Curro):
 - Make explicit that the thing about hardwiring NibCtrl+1 for the second
   compressed half is known to happen in Haswell and the issue with IVB
   might not be exactly the same.
 - Assign max_width instead of returning early so that we can handle
   multiple restrictions affecting to the same instruction.
 - Avoid division by 0 if the instruction does not write any registers.
 - Ignore instructions what have WE_all set.
 - Use the instruction execution type size instead of the dst type size.

v3 (Curro):
 - Move the implementation down so it is not placed in the middle of another
   workaround.
 - Declare channels_per_grf as const.
 - Don't break the loop early if we find a BAD_FILE source.
 - Fix the number of channels that the hardware shifts for the second half
   of a compressed instruction to be 8 in single precision and 4 in double
   precision.
---
 src/mesa/drivers/dri/i965/brw_fs.cpp | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 2f473cc..6ed98f5 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -4755,6 +4755,35 @@ get_fpu_lowered_simd_width(const struct brw_device_info *devinfo,
    if (inst->is_3src(devinfo) && !devinfo->supports_simd16_3src)
       max_width = MIN2(max_width, inst->exec_size / reg_count);
 
+   /* Pre-Gen8 EUs are hardwired to use the QtrCtrl+1 (where QtrCtrl is
+    * the 8-bit quarter of the execution mask signals specified in the
+    * instruction control fields) for the second compressed half of any
+    * single-precision instruction (for double-precision instructions
+    * it's hardwired to use NibCtrl+1, at least on HSW), which means that
+    * the EU will apply the wrong execution controls for the second
+    * sequential GRF write if the number of channels per GRF is not exactly
+    * eight in single-precision mode (or four in double-float mode).
+    *
+    * In this situation we calculate the maximum size of the split
+    * instructions so they only ever write to a single register.
+    */
+   if (devinfo->gen < 8 && inst->regs_written > 1 &&
+       !inst->force_writemask_all) {
+      const unsigned channels_per_grf = inst->exec_size / inst->regs_written;
+      unsigned exec_type_size = 0;
+      for (int i = 0; i < inst->sources; i++) {
+         if (inst->src[i].file != BAD_FILE)
+            exec_type_size = MAX2(exec_type_size, type_sz(inst->src[i].type));
+      }
+      assert(exec_type_size);
+
+      /* The hardware shifts exactly 8 channels per compressed half of the
+       * instruction in single-precision mode and exactly 4 in double-precision.
+       */
+      if (channels_per_grf != (exec_type_size == 8 ? 4 : 8))
+         max_width = MIN2(max_width, channels_per_grf);
+   }
+
    /* Only power-of-two execution sizes are representable in the instruction
     * control fields.
     */
-- 
2.7.4