From 5e76850ce38d1426417f68776623940f389ab373 Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Tue, 21 Jan 2025 18:35:24 +0100 Subject: [PATCH] pan/va: fix FAU validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validation was checking that if an instruction was accessing FAU RAM, only one 64-bit slot was accessed, and if it was accessing a FAU special value, only one was accessed, however it was not checking if both RAM and special were used, which is only allowed in messaging instructions except ATEST and BLEND. Fixes Piglit: spec/ati_fragment_shader/ati_fragment_shader-render-ops/mov c0.r Reviewed-by: Alejandro Piñeiro Reviewed-by: Mary Guillemard Reviewed-by: Lars-Ivar Hesselberg Simonsen Fixes: fd1906afea59 ("pan/va: Add FAU validation") Cc: mesa-stable Part-of: (cherry picked from commit e504825813d227100de4c8c07e83e9130b0bc831) --- .pick_status.json | 2 +- src/panfrost/compiler/valhall/va_validate.c | 58 +++++++++++++++------ 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 71d84951a98..bf694156601 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -904,7 +904,7 @@ "description": "pan/va: fix FAU validation", "nominated": true, "nomination_type": 2, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "fd1906afea59073780939810e0e46094264677d3", "notes": null diff --git a/src/panfrost/compiler/valhall/va_validate.c b/src/panfrost/compiler/valhall/va_validate.c index 0afa3dfccac..ffaecd2c069 100644 --- a/src/panfrost/compiler/valhall/va_validate.c +++ b/src/panfrost/compiler/valhall/va_validate.c @@ -60,18 +60,16 @@ fau_state_buffer(struct fau_state *fau, bi_index idx) return false; } +/* Instructions executed in the execution engine may only encode one FAU index. + * Instructions executed by the message unit may encode two FAU indices, + * except for ATEST and BLEND which may sometimes run in the execution engine. + */ static bool -fau_state_uniform(struct fau_state *fau, bi_index idx) +can_use_two_fau_indices(enum bi_opcode op) { - /* Each slot is 64-bits. The low/high half is encoded as the offset of the - * bi_index, which we want to ignore. - */ - unsigned slot = (idx.value & 63); - - if (fau->uniform_slot < 0) - fau->uniform_slot = slot; - - return fau->uniform_slot == slot; + return bi_opcode_props[op].message != BIFROST_MESSAGE_NONE && + op != BI_OPCODE_ATEST && + op != BI_OPCODE_BLEND; } static bool @@ -81,7 +79,32 @@ fau_is_special(enum bir_fau fau) } static bool -fau_state_special(struct fau_state *fau, bi_index idx) +fau_state_uniform(struct fau_state *fau, bi_index idx, enum bi_opcode op) +{ + /* Each slot is 64-bits. The low/high half is encoded as the offset of the + * bi_index, which we want to ignore. + */ + unsigned slot = (idx.value & 63); + + if (fau->uniform_slot < 0) + fau->uniform_slot = slot; + + if (fau->uniform_slot != slot) + return false; + + if (!can_use_two_fau_indices(op)) { + for (unsigned i = 0; i < ARRAY_SIZE(fau->buffer); ++i) { + bi_index buf = fau->buffer[i]; + if (!bi_is_null(buf) && fau_is_special(buf.value)) + return false; + } + } + + return true; +} + +static bool +fau_state_special(struct fau_state *fau, bi_index idx, enum bi_opcode op) { for (unsigned i = 0; i < ARRAY_SIZE(fau->buffer); ++i) { bi_index buf = fau->buffer[i]; @@ -91,11 +114,12 @@ fau_state_special(struct fau_state *fau, bi_index idx) return false; } - return true; + return fau->uniform_slot == -1 || can_use_two_fau_indices(op); } static bool -valid_src(struct fau_state *fau, unsigned fau_page, bi_index src) +valid_src(struct fau_state *fau, unsigned fau_page, bi_index src, + enum bi_opcode op) { if (src.type != BI_INDEX_FAU) return true; @@ -104,9 +128,9 @@ valid_src(struct fau_state *fau, unsigned fau_page, bi_index src) valid &= fau_state_buffer(fau, src); if (src.value & BIR_FAU_UNIFORM) - valid &= fau_state_uniform(fau, src); + valid &= fau_state_uniform(fau, src, op); else if (fau_is_special(src.value)) - valid &= fau_state_special(fau, src); + valid &= fau_state_special(fau, src, op); return valid; } @@ -119,7 +143,7 @@ va_validate_fau(bi_instr *I) unsigned fau_page = va_select_fau_page(I); bi_foreach_src(I, s) { - valid &= valid_src(&fau, fau_page, I->src[s]); + valid &= valid_src(&fau, fau_page, I->src[s], I->op); } return valid; @@ -135,7 +159,7 @@ va_repair_fau(bi_builder *b, bi_instr *I) struct fau_state push = fau; bi_index src = I->src[s]; - if (!valid_src(&fau, fau_page, src)) { + if (!valid_src(&fau, fau_page, src, I->op)) { bi_replace_src(I, s, bi_mov_i32(b, bi_strip_index(src))); /* Rollback update. Since the replacement move doesn't affect FAU