Mesa (18.1): i965/fs: unspills shoudn't use grf127 as dest since Gen8+
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Fri Jul 13 17:03:58 UTC 2018
Module: Mesa
Branch: 18.1
Commit: 251cf0dc3637ad170a6cf6917cb771ac37956a06
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=251cf0dc3637ad170a6cf6917cb771ac37956a06
Author: Jose Maria Casanova Crespo <jmcasanova at igalia.com>
Date: Wed Jul 11 11:19:20 2018 +0200
i965/fs: unspills shoudn't use grf127 as dest since Gen8+
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
assigned 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 assigns registers that
include the 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.
We also need to avoid that opt_bank_conflicts() optimization, that runs
after the register allocation, doesn't move things around, causing the
grf127 to be used in the condition we were avoiding.
Fixes piglit test tests/spec/arb_compute_shader/linker/bug-93840.shader_test
and some shader-db crashed because of the grf127 validation rule..
v2: make sure that opt_bank_conflicts() optimization doesn't change
the use of grf127. (Caio)
Found by Caio Marcelo de Oliveira Filho
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>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira at intel.com>
(cherry picked from commit 62f37ee53d9d5388eecef94369893b5467349306)
---
src/intel/compiler/brw_fs_bank_conflicts.cpp | 12 ++++++++++++
src/intel/compiler/brw_fs_reg_allocate.cpp | 27 ++++++++++++++++++++++-----
2 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/src/intel/compiler/brw_fs_bank_conflicts.cpp b/src/intel/compiler/brw_fs_bank_conflicts.cpp
index e87fcbfc5e..938ebcceac 100644
--- a/src/intel/compiler/brw_fs_bank_conflicts.cpp
+++ b/src/intel/compiler/brw_fs_bank_conflicts.cpp
@@ -540,6 +540,18 @@ namespace {
for (unsigned reg = 0; reg < 2; reg++)
constrained[p.atom_of_reg(reg)] = true;
+ /* At Intel Broadwell PRM, vol 07, section "Instruction Set Reference",
+ * subsection "EUISA Instructions", Send Message (page 990):
+ *
+ * "r127 must not be used for return address when there is a src and
+ * dest overlap in send instruction."
+ *
+ * Register allocation ensures that, so don't move 127 around to avoid
+ * breaking that property.
+ */
+ if (v->devinfo->gen >= 8)
+ constrained[p.atom_of_reg(127)] = true;
+
foreach_block_and_inst(block, fs_inst, inst, v->cfg) {
/* Assume that anything referenced via fixed GRFs is baked into the
* hardware's fixed-function logic and may be unsafe to move around.
diff --git a/src/intel/compiler/brw_fs_reg_allocate.cpp b/src/intel/compiler/brw_fs_reg_allocate.cpp
index 59e047483c..42ccb28de6 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,9 +671,26 @@ 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) {
+ /* Spilling instruction are genereated as SEND messages from MRF
+ * but as Gen7+ supports sending from GRF the driver will maps
+ * assingn these MRF registers to a GRF. Implementations reuses
+ * the dest of the send message as source. So as we will have an
+ * overlap for sure, we create an interference between destination
+ * and grf127.
+ */
+ 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);
}
}
}
More information about the mesa-commit
mailing list