summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJuan A. Suarez Romero <jasuarez@igalia.com>2021-09-08 10:04:45 +0200
committerDylan Baker <dylan.c.baker@intel.com>2021-09-15 09:33:22 -0700
commitddb5243ccd3e33a9b7aaf5d88c97ad32fe437572 (patch)
tree3315dd25129fc08c5b59ef0ea348bdc19125a1d3
parentbf7543fcefa04c5aed29583a496de1355bf0e233 (diff)
broadcom/compiler: force a last thrsw for spilling
As we don't know if we are going to have spilling or not, emit always a last thrsw at the end of the shader. If later we don't have spillings and we don't need that last thrsw, we remove it and switch back to the previous one. This way we ensure all the spilling happens always before the last thrsw. v2 (Juan): - Rework the code to force a last thrsw and remove later if no spilling v3: - Merge functionality inside vir_emit_last_thrsw (Iago) - Add vir_restore_last_thrsw (Juan) v4 (Iago): - Fix/add new comments - Rename variables/parameters v5 (Iago): - Fix comments - Add assertion Cc: mesa-stable Fixes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4760 Reviewed-by: Iago Toral Quiroga <itoral@igalia.com> Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12322> (cherry picked from commit c98ddc778a34683e55d29122edba53ac4969deea) Conflicts: src/broadcom/compiler/nir_to_vir.c
-rw-r--r--.pick_status.json2
-rw-r--r--src/broadcom/compiler/nir_to_vir.c62
-rw-r--r--src/broadcom/compiler/v3d_compiler.h1
-rw-r--r--src/broadcom/compiler/vir_register_allocate.c20
4 files changed, 61 insertions, 24 deletions
diff --git a/.pick_status.json b/.pick_status.json
index 3035d562be3..4cf1ff0b2ce 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -1984,7 +1984,7 @@
"description": "broadcom/compiler: force a last thrsw for spilling",
"nominated": true,
"nomination_type": 0,
- "resolution": 0,
+ "resolution": 1,
"main_sha": null,
"because_sha": null
},
diff --git a/src/broadcom/compiler/nir_to_vir.c b/src/broadcom/compiler/nir_to_vir.c
index 2a4dac723f6..223e7f00cc8 100644
--- a/src/broadcom/compiler/nir_to_vir.c
+++ b/src/broadcom/compiler/nir_to_vir.c
@@ -3900,9 +3900,25 @@ vir_remove_thrsw(struct v3d_compile *c)
c->last_thrsw = NULL;
}
-void
-vir_emit_last_thrsw(struct v3d_compile *c)
+/**
+ * This makes sure we have a top-level last thread switch which signals the
+ * start of the last thread section, which may include adding a new thrsw
+ * instruction if needed. We don't allow spilling in the last thread section, so
+ * if we need to do any spills that inject additional thread switches later on,
+ * we ensure this thread switch will still be the last thread switch in the
+ * program, which makes last thread switch signalling a lot easier when we have
+ * spilling. If in the end we don't need to spill to compile the program and we
+ * injected a new thread switch instruction here only for that, we will
+ * eventually restore the previous last thread switch and remove the one we
+ * added here.
+ */
+static void
+vir_emit_last_thrsw(struct v3d_compile *c,
+ struct qinst **restore_last_thrsw,
+ bool *restore_scoreboard_lock)
{
+ *restore_last_thrsw = c->last_thrsw;
+
/* On V3D before 4.1, we need a TMU op to be outstanding when thread
* switching, so disable threads if we didn't do any TMU ops (each of
* which would have emitted a THRSW).
@@ -3911,7 +3927,7 @@ vir_emit_last_thrsw(struct v3d_compile *c)
c->threads = 1;
if (c->last_thrsw)
vir_remove_thrsw(c);
- return;
+ *restore_last_thrsw = NULL;
}
/* If we're threaded and the last THRSW was in conditional code, then
@@ -3934,8 +3950,34 @@ vir_emit_last_thrsw(struct v3d_compile *c)
vir_emit_thrsw(c);
}
+ /* If we have not inserted a last thread switch yet, do it now to ensure
+ * any potential spilling we do happens before this. If we don't spill
+ * in the end, we will restore the previous one.
+ */
+ if (*restore_last_thrsw == c->last_thrsw) {
+ if (*restore_last_thrsw)
+ (*restore_last_thrsw)->is_last_thrsw = false;
+ *restore_scoreboard_lock = c->lock_scoreboard_on_first_thrsw;
+ vir_emit_thrsw(c);
+ } else {
+ *restore_last_thrsw = c->last_thrsw;
+ }
+
+ assert(c->last_thrsw);
+ c->last_thrsw->is_last_thrsw = true;
+}
+
+static void
+vir_restore_last_thrsw(struct v3d_compile *c,
+ struct qinst *thrsw,
+ bool scoreboard_lock)
+{
+ assert(c->last_thrsw);
+ vir_remove_instruction(c, c->last_thrsw);
+ c->last_thrsw = thrsw;
if (c->last_thrsw)
c->last_thrsw->is_last_thrsw = true;
+ c->lock_scoreboard_on_first_thrsw = scoreboard_lock;
}
/* There's a flag in the shader for "center W is needed for reasons other than
@@ -3973,8 +4015,14 @@ v3d_nir_to_vir(struct v3d_compile *c)
nir_to_vir(c);
+ bool restore_scoreboard_lock = false;
+ struct qinst *restore_last_thrsw;
+
/* Emit the last THRSW before STVPM and TLB writes. */
- vir_emit_last_thrsw(c);
+ vir_emit_last_thrsw(c,
+ &restore_last_thrsw,
+ &restore_scoreboard_lock);
+
switch (c->s->info.stage) {
case MESA_SHADER_FRAGMENT:
@@ -4073,6 +4121,12 @@ v3d_nir_to_vir(struct v3d_compile *c)
vir_remove_thrsw(c);
}
+ /* If we didn't spill, then remove the last thread switch we injected
+ * artificially (if any) and restore the previous one.
+ */
+ if (!c->spills && c->last_thrsw != restore_last_thrsw)
+ vir_restore_last_thrsw(c, restore_last_thrsw, restore_scoreboard_lock);
+
if (c->spills &&
(V3D_DEBUG & (V3D_DEBUG_VIR |
v3d_debug_flag_for_shader_stage(c->s->info.stage)))) {
diff --git a/src/broadcom/compiler/v3d_compiler.h b/src/broadcom/compiler/v3d_compiler.h
index c1ea346bead..23a04c24dd9 100644
--- a/src/broadcom/compiler/v3d_compiler.h
+++ b/src/broadcom/compiler/v3d_compiler.h
@@ -1051,7 +1051,6 @@ void vir_set_unpack(struct qinst *inst, int src,
void vir_set_pack(struct qinst *inst, enum v3d_qpu_output_pack pack);
struct qreg vir_get_temp(struct v3d_compile *c);
-void vir_emit_last_thrsw(struct v3d_compile *c);
void vir_calculate_live_intervals(struct v3d_compile *c);
int vir_get_nsrc(struct qinst *inst);
bool vir_has_side_effects(struct v3d_compile *c, struct qinst *inst);
diff --git a/src/broadcom/compiler/vir_register_allocate.c b/src/broadcom/compiler/vir_register_allocate.c
index c55d8dc937d..60535c17a4b 100644
--- a/src/broadcom/compiler/vir_register_allocate.c
+++ b/src/broadcom/compiler/vir_register_allocate.c
@@ -251,7 +251,7 @@ v3d_spill_reg(struct v3d_compile *c, int spill_temp)
}
struct qinst *last_thrsw = c->last_thrsw;
- assert(!last_thrsw || last_thrsw->is_last_thrsw);
+ assert(last_thrsw && last_thrsw->is_last_thrsw);
int start_num_temps = c->num_temps;
@@ -337,29 +337,13 @@ v3d_spill_reg(struct v3d_compile *c, int spill_temp)
spill_offset);
}
}
-
- /* If we didn't have a last-thrsw inserted by nir_to_vir and
- * we've been inserting thrsws, then insert a new last_thrsw
- * right before we start the vpm/tlb sequence for the last
- * thread segment.
- */
- if (!is_uniform && !last_thrsw && c->last_thrsw &&
- (v3d_qpu_writes_vpm(&inst->qpu) ||
- v3d_qpu_uses_tlb(&inst->qpu))) {
- c->cursor = vir_before_inst(inst);
- vir_emit_thrsw(c);
-
- last_thrsw = c->last_thrsw;
- last_thrsw->is_last_thrsw = true;
- }
}
}
/* Make sure c->last_thrsw is the actual last thrsw, not just one we
* inserted in our most recent unspill.
*/
- if (last_thrsw)
- c->last_thrsw = last_thrsw;
+ c->last_thrsw = last_thrsw;
/* Don't allow spilling of our spilling instructions. There's no way
* they can help get things colored.