[Mesa-dev] [PATCH] i965/fs/generator: Don't use the address immediate for MOV_INDIRECT

[Jason Ekstrand]

The address immediate field is only 9 bits and, since the value is in
bytes, the highest GRF we can point to with it is g15.  This makes it
pretty close to useless for MOV_INDIRECT.  There were already piles of
restrictions preventing us from using it prior to Broadwell, so let's get
rid of the gen8+ code path entirely.

Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97779
Cc: "12.0 13.0" <mesa-stable at lists.freedesktop.org>
---
 src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 55 +++++++++++++-------------
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
index d25d26a..7130bf5 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
@@ -386,33 +386,34 @@ fs_generator::generate_mov_indirect(fs_inst *inst,
          retype(spread(indirect_byte_offset, 2), BRW_REGISTER_TYPE_UW);
 
       struct brw_reg ind_src;
-      if (devinfo->gen < 8) {
-         /* From the Haswell PRM section "Register Region Restrictions":
-          *
-          *    "The lower bits of the AddressImmediate must not overflow to
-          *    change the register address.  The lower 5 bits of Address
-          *    Immediate when added to lower 5 bits of address register gives
-          *    the sub-register offset. The upper bits of Address Immediate
-          *    when added to upper bits of address register gives the register
-          *    address. Any overflow from sub-register offset is dropped."
-          *
-          * This restriction is only listed in the Haswell PRM but emperical
-          * testing indicates that it applies on all older generations and is
-          * lifted on Broadwell.
-          *
-          * Since the indirect may cause us to cross a register boundary, this
-          * makes the base offset almost useless.  We could try and do
-          * something clever where we use a actual base offset if
-          * base_offset % 32 == 0 but that would mean we were generating
-          * different code depending on the base offset.  Instead, for the
-          * sake of consistency, we'll just do the add ourselves.
-          */
-         brw_ADD(p, addr, indirect_byte_offset, brw_imm_uw(imm_byte_offset));
-         ind_src = brw_VxH_indirect(0, 0);
-      } else {
-         brw_MOV(p, addr, indirect_byte_offset);
-         ind_src = brw_VxH_indirect(0, imm_byte_offset);
-      }
+
+      /* There are a number of reasons why we don't use the base offset here.
+       * One reason is that the field is only 9 bits which means we can only
+       * use it on the first 16 GRFs.  Also, from the Haswell PRM section
+       * "Register Region Restrictions":
+       *
+       *    "The lower bits of the AddressImmediate must not overflow to
+       *    change the register address.  The lower 5 bits of Address
+       *    Immediate when added to lower 5 bits of address register gives
+       *    the sub-register offset. The upper bits of Address Immediate
+       *    when added to upper bits of address register gives the register
+       *    address. Any overflow from sub-register offset is dropped."
+       *
+       * Since the indirect may cause us to cross a register boundary, this
+       * makes the base offset almost useless.  We could try and do something
+       * clever where we use a actual base offset if base_offset % 32 == 0 but
+       * that would mean we were generating different code depending on the
+       * base offset.  Instead, for the sake of consistency, we'll just do the
+       * add ourselves.  This restriction is only listed in the Haswell PRM
+       * but emperical testing indicates that it applies on all older
+       * generations and is lifted on Broadwell.
+       *
+       * In the end, while base_offset is nice to look at in the generated
+       * code, using it saves us 0 instructions and would require quite a bit
+       * of case-by-case work.  It's just not worth it.
+       */
+      brw_ADD(p, addr, indirect_byte_offset, brw_imm_uw(imm_byte_offset));
+      ind_src = brw_VxH_indirect(0, 0);
 
       brw_inst *mov = brw_MOV(p, dst, retype(ind_src, dst.type));
 
-- 
2.5.0.400.gff86faf