broadcom/compiler: only patch temps that existed before the current spill

When we spill we add new temps. We should be careful not to access
liveness for these until we have re-computed it after all spills and
fill for that the spilled temp have been processed so as to avoid
out-of-bounds accesses to the c->temp_start and c->temp_end arrays.

This fixes a crash in a Three.js demo when we try to patch register
classes after a TMU spill that was caused because we would incorrectly
try to patch the same temps we had just added for the spill itself,
which is not only unnecessary but also incorrect since we these temps
would not have liveness information available yet and thus would
cause out of bounds accesses.

Fixes: f3c3228522 ('broadcom/compiler: do not rebuild the interference graph after each spill')
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15107>
This commit is contained in:
Iago Toral Quiroga 2022-02-21 14:09:33 +01:00 committed by Marge Bot
parent 7ec8a3205e
commit a4b164b57b
2 changed files with 10 additions and 4 deletions

View File

@ -845,6 +845,11 @@ struct v3d_compile {
struct qreg undef;
uint32_t num_temps;
/* Number of temps in the program right before we spill a new temp. We
* use this to know which temps existed before a spill and which were
* added with the spill itself.
*/
uint32_t spill_start_num_temps;
struct vir_cursor cursor;
struct list_head blocks;

View File

@ -356,7 +356,7 @@ v3d_emit_spill_tmua(struct v3d_compile *c,
* is not affected (and only the spilled temp starts at ip). Something
* that ends at ip won't be affected either.
*/
for (int i = 0; i < c->num_temps; i++) {
for (int i = 0; i < c->spill_start_num_temps; i++) {
bool thrsw_cross = fill_dst ?
c->temp_start[i] < ip && c->temp_end[i] >= ip :
c->temp_start[i] < ip && c->temp_end[i] > ip;
@ -414,7 +414,7 @@ interferes(int32_t t0_start, int32_t t0_end, int32_t t1_start, int32_t t1_end)
static void
v3d_spill_reg(struct v3d_compile *c, int *acc_nodes, int spill_temp)
{
int start_num_temps = c->num_temps;
c->spill_start_num_temps = c->num_temps;
c->spilling = true;
bool is_uniform = vir_is_mov_uniform(c, spill_temp);
@ -564,7 +564,7 @@ v3d_spill_reg(struct v3d_compile *c, int *acc_nodes, int spill_temp)
/* Don't allow spilling of our spilling instructions. There's no way
* they can help get things colored.
*/
for (int i = start_num_temps; i < c->num_temps; i++)
for (int i = c->spill_start_num_temps; i < c->num_temps; i++)
BITSET_CLEAR(c->spillable, i);
/* Reset interference for spilled node */
@ -592,7 +592,8 @@ v3d_spill_reg(struct v3d_compile *c, int *acc_nodes, int spill_temp)
uint32_t node = c->ra_map.temp[i].node;
c->ra_map.node[node].priority = c->temp_end[i] - c->temp_start[i];
for (uint32_t j = MAX2(i + 1, start_num_temps); j < c->num_temps; j++) {
for (uint32_t j = MAX2(i + 1, c->spill_start_num_temps);
j < c->num_temps; j++) {
if (interferes(c->temp_start[i], c->temp_end[i],
c->temp_start[j], c->temp_end[j])) {
ra_add_node_interference(c->g,