From 1b42bc76daf10b968409471e5829173e97ae297c Mon Sep 17 00:00:00 2001 From: Connor Abbott Date: Mon, 2 Dec 2024 12:16:41 -0500 Subject: [PATCH] ir3: Fix reload_live_out() in shared RA We never split live ranges, so we don't need to store the location of each live value when recording live outs, but the physreg assigned to a register will still be clobbered when we reload it so we have to record the original physreg and then make sure to use it when reloading the live out. We probably never encountered a case where we needed to reload live outs in a loop before, but after enabling clustered subgroup reductions dEQP-VK.subgroups.clustered.compute.subgroupclusteredmin_{i,u}64vec4_requiredsubgroupsize hits this case and fails in RA validation without this fix. Fixes: fa22b0901af ("ir3/ra: Add specialized shared register RA/spilling") Part-of: --- src/freedreno/ir3/ir3_shared_ra.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/freedreno/ir3/ir3_shared_ra.c b/src/freedreno/ir3/ir3_shared_ra.c index b02bb09a473..f0ec2133b60 100644 --- a/src/freedreno/ir3/ir3_shared_ra.c +++ b/src/freedreno/ir3/ir3_shared_ra.c @@ -30,6 +30,11 @@ struct ra_interval { struct rb_node physreg_node; physreg_t physreg_start, physreg_end; + /* If this interval was spilled, the original physreg_start before spilling. + * Used when reloading live outs. + */ + physreg_t physreg_start_orig; + /* Where the shared register is spilled to. If there were no uses when it's * spilled it could be the original defining instruction. */ @@ -438,6 +443,7 @@ spill_interval_children(struct ra_interval *interval, interval->interval.reg->interval_start) / reg_elem_size(interval->interval.reg), reg_elems(child->interval.reg), before); + interval->physreg_start_orig = child->physreg_start; } spill_interval_children(child, before); } @@ -481,6 +487,7 @@ spill_interval(struct ra_ctx *ctx, struct ra_interval *interval) (interval->interval.reg->flags & IR3_REG_HALF) ? TYPE_U16 : TYPE_U32; interval->spill_def = dst; + interval->physreg_start_orig = interval->physreg_start; } spill_interval_children(interval, interval->spill_def->instr); @@ -1138,6 +1145,16 @@ reload_live_outs(struct ra_ctx *ctx, struct ir3_block *block) struct ra_interval *interval = &ctx->intervals[name]; if (!interval->interval.inserted) { d("reloading %d at end of backedge", reg->name); + + /* When this interval was spilled inside the loop, we probably chose a + * different physreg for it than the original physreg when it was + * defined outside the loop. Restore the original physreg so that we + * spill it correctly. + */ + unsigned size = interval->physreg_end - interval->physreg_start; + interval->physreg_start = interval->physreg_start_orig; + interval->physreg_end = interval->physreg_start + size; + reload_interval(ctx, ir3_before_terminator(block), interval); } }