[Mesa-dev] [PATCH] i965/fs: unspills shoudn't use grf127 as dest since Gen8+

Jose Maria Casanova Crespo jmcasanova at igalia.com
Wed Jul 11 16:03:05 UTC 2018


At 232ed8980217dd65ab0925df28156f565b94b2e5 "i965/fs: Register allocator
shoudn't use grf127 for sends dest" we didn't take into account the case
of SEND instructions that are not send_from_grf. But since Gen7+ although
the backend still uses MRFs internally for sends they are finally asigned
to a GRFs.

In the case of unspills the backend assigns directly as source its
destination because it is suppose to be available. So we always have a
source-destination overlap. If the reg_allocator asigns registers that
include de grf127 we fail the validation rule that affects Gen8+
"r127 must not be used for return address when there is a src and dest
overlap in send instruction."

So this patch activates the grf127_send_hack_node for Gen8+ and if we have
any register spilled we add interferences to the destination of the unspill
operations.

Found by Caio Marcelo de Oliveira Filho

Fixes piglit test tests/spec/arb_compute_shader/linker/bug-93840.shader_test

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107193
Fixes: 232ed89802 "i965/fs: Register allocator shoudn't use grf127 for sends dest"
Cc: 18.1 <mesa-stable at lists.freedesktop.org>
Cc: Caio Marcelo de Oliveira Filho <caio.oliveira at intel.com>
Cc: Jason Ekstrand <jason at jlekstrand.net>
---
 src/intel/compiler/brw_fs_reg_allocate.cpp | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/intel/compiler/brw_fs_reg_allocate.cpp b/src/intel/compiler/brw_fs_reg_allocate.cpp
index 59e047483c0..3ea2e7547c6 100644
--- a/src/intel/compiler/brw_fs_reg_allocate.cpp
+++ b/src/intel/compiler/brw_fs_reg_allocate.cpp
@@ -549,7 +549,7 @@ fs_visitor::assign_regs(bool allow_spilling, bool spill_all)
    if (devinfo->gen >= 7)
       node_count += BRW_MAX_GRF - GEN7_MRF_HACK_START;
    int grf127_send_hack_node = node_count;
-   if (devinfo->gen >= 8 && dispatch_width == 8)
+   if (devinfo->gen >= 8)
       node_count ++;
    struct ra_graph *g =
       ra_alloc_interference_graph(compiler->fs_reg_sets[rsi].regs, node_count);
@@ -656,7 +656,7 @@ fs_visitor::assign_regs(bool allow_spilling, bool spill_all)
       }
    }
 
-   if (devinfo->gen >= 8 && dispatch_width == 8) {
+   if (devinfo->gen >= 8) {
       /* At Intel Broadwell PRM, vol 07, section "Instruction Set Reference",
        * subsection "EUISA Instructions", Send Message (page 990):
        *
@@ -671,13 +671,25 @@ fs_visitor::assign_regs(bool allow_spilling, bool spill_all)
        * overlap between sources and destination.
        */
       ra_set_node_reg(g, grf127_send_hack_node, 127);
-      foreach_block_and_inst(block, fs_inst, inst, cfg) {
-         if (inst->is_send_from_grf() && inst->dst.file == VGRF) {
-            ra_add_node_interference(g, inst->dst.nr, grf127_send_hack_node);
+      if (dispatch_width == 8) {
+         foreach_block_and_inst(block, fs_inst, inst, cfg) {
+            if (inst->is_send_from_grf() && inst->dst.file == VGRF)
+               ra_add_node_interference(g, inst->dst.nr, grf127_send_hack_node);
+         }
+      }
+
+      if (spilled_any_registers) {
+         foreach_block_and_inst(block, fs_inst, inst, cfg) {
+            if ((inst->opcode == SHADER_OPCODE_GEN7_SCRATCH_READ ||
+                inst->opcode == SHADER_OPCODE_GEN4_SCRATCH_READ) &&
+                inst->dst.file ==VGRF) {
+               ra_add_node_interference(g, inst->dst.nr, grf127_send_hack_node);
+            }
          }
       }
    }
 
+
    /* Debug of register spilling: Go spill everything. */
    if (unlikely(spill_all)) {
       int reg = choose_spill_reg(g);
-- 
2.17.1



More information about the mesa-dev mailing list