ir3: only add live-in phis for top-level intervals while spilling

When both an interval and some of its children would be live-in, we used
to add phis for all of them. This could lead to cases where the pressure
after spilling was higher than before.

This happens, for example, when both a split and its parent are live-in.
Before spilling, the split wouldn't add to the pressure because its
parent had already been inserted. After spilling, since we created a phi
for the split, the link with its parent would be lost and it would add
to the pressure.

Fix this by only adding phis for top-level intervals and adding splits
after them.

Fixes: 613eaac7b5 ("ir3: Initial support for spilling non-shared registers")
Signed-off-by: Job Noorman <jnoorman@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29497>
(cherry picked from commit 6bc7cd6108)
This commit is contained in:
Job Noorman 2024-06-17 11:34:26 +02:00 committed by Eric Engestrom
parent 0c8177d28e
commit 1a23ca0dfa
2 changed files with 47 additions and 21 deletions

View file

@ -114,7 +114,7 @@
"description": "ir3: only add live-in phis for top-level intervals while spilling",
"nominated": true,
"nomination_type": 1,
"resolution": 0,
"resolution": 1,
"main_sha": null,
"because_sha": "613eaac7b53bfbfcd6ef536412be6c9c63cdea4f",
"notes": null

View file

@ -1031,6 +1031,17 @@ static void
handle_instr(struct ra_spill_ctx *ctx, struct ir3_instruction *instr)
{
ra_foreach_dst (dst, instr) {
/* No need to handle instructions inserted while spilling. Most are
* ignored automatically by virtue of being inserted before the current
* instruction. However, for live-ins, we may insert extracts after the
* phis. Trying to handle them ends badly as they don't have intervals
* allocated.
* Note: since liveness is calculated before spilling, original
* instruction have a name while new ones don't.
*/
if (!dst->name)
return;
init_dst(ctx, dst);
}
@ -1477,20 +1488,6 @@ live_in_rewrite(struct ra_spill_ctx *ctx,
if (def)
_mesa_hash_table_insert(state->remap, def, new_val);
rb_tree_foreach (struct ra_spill_interval, child,
&interval->interval.children, interval.node) {
assert(new_val->flags & IR3_REG_SSA);
struct ir3_register *child_def =
extract(new_val->def,
(child->interval.reg->interval_start - def->interval_start) /
reg_elem_size(def), reg_elems(child->interval.reg),
ir3_before_terminator(pred));
struct reg_or_immed *child_val = ralloc(ctx, struct reg_or_immed);
child_val->def = child_def;
child_val->flags = child_def->flags;
live_in_rewrite(ctx, child, child_val, block, pred_idx);
}
}
static void
@ -1532,7 +1529,7 @@ reload_live_ins(struct ra_spill_ctx *ctx, struct ir3_block *block)
static void
add_live_in_phi(struct ra_spill_ctx *ctx, struct ir3_register *def,
struct ir3_block *block)
struct ir3_register *parent_def, struct ir3_block *block)
{
struct ra_spill_interval *interval = ctx->intervals[def->name];
if (!interval->interval.inserted)
@ -1565,6 +1562,26 @@ add_live_in_phi(struct ra_spill_ctx *ctx, struct ir3_register *def,
if (!needs_phi) {
interval->dst.def = cur_def;
interval->dst.flags = cur_def->flags;
rb_tree_foreach (struct ra_spill_interval, child,
&interval->interval.children, interval.node) {
add_live_in_phi(ctx, child->interval.reg, cur_def, block);
}
return;
}
if (parent_def) {
/* We have a child interval that needs a phi but whose parent does not
* need one (otherwise parent_def would be NULL). Just extract the child
* from the parent without creating a phi for the child.
*/
unsigned offset = (def->interval_start - parent_def->interval_start) /
reg_elem_size(def);
struct ir3_register *extracted =
extract(parent_def, offset, reg_elems(def), ir3_after_phis(block));
rewrite_src_interval(ctx, interval, extracted,
ir3_after_instr(extracted->instr));
return;
}
@ -1600,6 +1617,19 @@ add_live_in_phi(struct ra_spill_ctx *ctx, struct ir3_register *def,
interval->dst.def = dst;
interval->dst.flags = dst->flags;
rewrite_src_interval(ctx, interval, dst, ir3_after_phis(block));
}
static void
add_live_in_phis(struct ra_spill_ctx *ctx, struct ir3_block *block)
{
rb_tree_foreach (struct ra_spill_interval, interval, &ctx->reg_ctx.intervals,
interval.node) {
if (BITSET_TEST(ctx->live->live_in[block->index],
interval->interval.reg->name)) {
add_live_in_phi(ctx, interval->interval.reg, NULL, block);
}
}
}
/* When spilling a block with a single predecessors, the pred may have other
@ -1831,11 +1861,7 @@ handle_block(struct ra_spill_ctx *ctx, struct ir3_block *block)
break;
rewrite_phi(ctx, instr, block);
}
BITSET_FOREACH_SET (name, ctx->live->live_in[block->index],
ctx->live->definitions_count) {
struct ir3_register *reg = ctx->live->definitions[name];
add_live_in_phi(ctx, reg, block);
}
add_live_in_phis(ctx, block);
}
} else {
update_max_pressure(ctx);