From e4520b1ddaafa9fd62ebd3cef1df4cf0347a3a26 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 14 Jan 2026 09:37:23 -0500 Subject: [PATCH] agx: fix SSA repair with phis with constants For large constants inlined into phis, this would overread the remap[] array, which could crash. No CTS tests affected though. Christoph found the bug and fixed it for Bifrost over in https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39305. I just did a quick CTS run of the obvious AGX backport over this morning's breakfast. Cc: mesa-stable Signed-off-by: Alyssa Rosenzweig Reported-by: Christoph Pillmayer Part-of: --- src/asahi/compiler/agx_repair_ssa.c | 34 ++++++++++++++++------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/asahi/compiler/agx_repair_ssa.c b/src/asahi/compiler/agx_repair_ssa.c index 7864e7716bd..3e7a2c23efb 100644 --- a/src/asahi/compiler/agx_repair_ssa.c +++ b/src/asahi/compiler/agx_repair_ssa.c @@ -176,8 +176,24 @@ agx_opt_trivial_phi(agx_context *ctx) bool all_same = true; agx_foreach_src(phi, s) { - /* TODO: Handle cycles faster */ - if (!agx_is_null(remap[phi->src[s].value])) { + /* Only optimize trivial phis with normal sources. It is possible + * to optimize something like `phi #0, #0` but... + * + * 1. It would inadvently propagate constants which may be + * invalid. Copyprop knows the rules for this, but we don't here. + * + * 2. These trivial phis should be optimized at the NIR level. + * This pass is just to clean up spilling. + * + * So skip them for correctness in case NIR misses something + * (which can happen depending on pass order). + * + * --- + * + * TODO: Handle cycles faster. + */ + if (phi->src[s].type != AGX_INDEX_NORMAL || + agx_is_null(remap[phi->src[s].value])) { all_same = false; break; } @@ -195,19 +211,7 @@ agx_opt_trivial_phi(agx_context *ctx) same = phi->src[s]; } - /* Only optimize trivial phis with normal sources. It is possible - * to optimize something like `phi #0, #0` but... - * - * 1. It would inadvently propagate constants which may be invalid. - * Copyprop knows the rules for this, but we don't here. - * - * 2. These trivial phis should be optimized at the NIR level. This - * pass is just to clean up spilling. - * - * So skip them for correctness in case NIR misses something (which - * can happen depending on pass order). - */ - if (all_same && same.type == AGX_INDEX_NORMAL) { + if (all_same) { remap[phi->dest[0].value] = same; agx_remove_instruction(phi); progress = true;