From 6c776793ba438d68bb8aabece554b7f8fc0f1576 Mon Sep 17 00:00:00 2001 From: Job Noorman Date: Fri, 29 Nov 2024 08:38:31 +0100 Subject: [PATCH] ir3/ra: fix non-trivial collect detection Detecting non-trivial collects after the fact, i.e., once they are created by a register assignment of their dst, does not work as this may cause two different intervals to share a physreg. For example: _meta:collect sssa_361:150(r50.x) (wrmask=0xf), sssa_19:12(r50.x), sssa_103:13(r50.y), sssa_355:102(r51.z), sssa_356:103(r51.w) This is a non-trivial collect with a partial overlap with one of its child intervals. After moving its dst to a new interval, it will have the same physreg as the existing interval for sssa_19, causing all sorts of trouble for RA. Prevent this by detecting that a future collect may become non-trivial at the moment one of its sources gets a register assignment that does not correspond with it merge set's preferred reg and allocating a new interval for this component. Signed-off-by: Job Noorman Fixes: b36a7ce0f19 ("ir3/ra: prevent moving source intervals for shared collects") Part-of: --- src/freedreno/ir3/ir3_shared_ra.c | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/src/freedreno/ir3/ir3_shared_ra.c b/src/freedreno/ir3/ir3_shared_ra.c index f0ec2133b60..393be4c3bfc 100644 --- a/src/freedreno/ir3/ir3_shared_ra.c +++ b/src/freedreno/ir3/ir3_shared_ra.c @@ -826,24 +826,6 @@ assign_src(struct ra_ctx *ctx, struct ir3_register *src) interval->src = false; } -static bool -is_nontrivial_collect(struct ir3_instruction *collect) -{ - if (collect->opc != OPC_META_COLLECT) { - return false; - } - - struct ir3_register *dst = collect->dsts[0]; - - foreach_src_n (src, src_n, collect) { - if (src->num != dst->num + src_n) { - return true; - } - } - - return false; -} - static void handle_dst(struct ra_ctx *ctx, struct ir3_instruction *instr, struct ir3_register *dst) @@ -886,11 +868,15 @@ handle_dst(struct ra_ctx *ctx, struct ir3_instruction *instr, * sources don't line-up with the destination) may cause source intervals to * get implicitly moved when they are inserted as children of the destination * interval. Since we don't support moving intervals in shared RA, this may - * cause illegal register allocations. Prevent this by creating a new - * top-level interval for the destination so that the source intervals will - * be left alone. + * cause illegal register allocations. Prevent this by making sure + * non-trivial collects will not share a merge set with (and will not be a + * parent interval of) their components. Detect this by checking if a dst got + * a register assignment that does not correspond with the existing preferred + * reg of its merge set, as this might cause a future collect covering its + * interval to become non-trivial. */ - if (is_nontrivial_collect(instr)) { + if (dst->merge_set && dst->merge_set->preferred_reg != (physreg_t)~0 && + physreg != dst->merge_set->preferred_reg + dst->merge_set_offset) { dst->merge_set = NULL; dst->interval_start = ctx->live->interval_offset; dst->interval_end = dst->interval_start + reg_size(dst);