From 33fef27356a508be719156c65e9cd7100ea61b30 Mon Sep 17 00:00:00 2001 From: Mary Guillemard Date: Fri, 2 Aug 2024 09:42:31 +0200 Subject: [PATCH] bi: Do not mark tex ops as skip when dest is used by control flow Previously, it was possible to have a texture operation marked as SKIP while one of the dests was in use in conditional control flow. If an helper thread was to execute that instruction, it would result in an undefined value being used. This fix "dEQP-VK.graphicsfuzz.cov-nested-loops-sample-opposite-corners" where helper threads would get stuck inside a loop depending on the result of a TEXS_2D invocation. Signed-off-by: Mary Guillemard Reviewed-by: Boris Brezillon Acked-by: Alyssa Rosenzweig Part-of: --- src/panfrost/compiler/bi_helper_invocations.c | 46 ++++++++++++++++--- src/panfrost/compiler/bifrost_compile.c | 4 +- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/panfrost/compiler/bi_helper_invocations.c b/src/panfrost/compiler/bi_helper_invocations.c index f266cbf172f..f9269ebe80d 100644 --- a/src/panfrost/compiler/bi_helper_invocations.c +++ b/src/panfrost/compiler/bi_helper_invocations.c @@ -110,6 +110,39 @@ bi_instr_uses_helpers(bi_instr *I) } } +static void +bi_add_branch_compare_values(const bi_instr *I, BITSET_WORD *deps) +{ + switch (I->op) { + case BI_OPCODE_BRANCHZI: + case BI_OPCODE_BRANCHC_I16: + case BI_OPCODE_BRANCHC_I32: + BITSET_SET(deps, I->src[0].value); + break; + case BI_OPCODE_BRANCH_F16: + case BI_OPCODE_BRANCH_F32: + case BI_OPCODE_BRANCH_I16: + case BI_OPCODE_BRANCH_I32: + case BI_OPCODE_BRANCH_S16: + case BI_OPCODE_BRANCH_S32: + case BI_OPCODE_BRANCH_U16: + case BI_OPCODE_BRANCH_U32: + case BI_OPCODE_BRANCHZ_F16: + case BI_OPCODE_BRANCHZ_F32: + case BI_OPCODE_BRANCHZ_I16: + case BI_OPCODE_BRANCHZ_I32: + case BI_OPCODE_BRANCHZ_S16: + case BI_OPCODE_BRANCHZ_S32: + case BI_OPCODE_BRANCHZ_U16: + case BI_OPCODE_BRANCHZ_U32: + BITSET_SET(deps, I->src[0].value); + BITSET_SET(deps, I->src[1].value); + break; + default: + break; + } +} + /* Does a block use helpers directly */ static bool bi_block_uses_helpers(bi_block *block) @@ -224,14 +257,15 @@ bi_analyze_helper_requirements(bi_context *ctx) BITSET_WORD *deps = calloc(sizeof(BITSET_WORD), ctx->ssa_alloc); /* Initialize with the sources of instructions consuming - * derivatives */ + * derivatives and the sources of conditional branch instructions */ bi_foreach_instr_global(ctx, I) { - if (!bi_instr_uses_helpers(I)) - continue; - - bi_foreach_ssa_src(I, s) - BITSET_SET(deps, I->src[s].value); + if (bi_instr_uses_helpers(I)) { + bi_foreach_ssa_src(I, s) + BITSET_SET(deps, I->src[s].value); + } else { + bi_add_branch_compare_values(I, deps); + } } /* Propagate that up */ diff --git a/src/panfrost/compiler/bifrost_compile.c b/src/panfrost/compiler/bifrost_compile.c index 3c53a08a627..02df15144b2 100644 --- a/src/panfrost/compiler/bifrost_compile.c +++ b/src/panfrost/compiler/bifrost_compile.c @@ -5180,8 +5180,10 @@ bi_compile_variant_nir(nir_shader *nir, * under valid scheduling. Helpers are only defined for fragment * shaders, so this analysis is only required in fragment shaders. */ - if (ctx->stage == MESA_SHADER_FRAGMENT) + if (ctx->stage == MESA_SHADER_FRAGMENT) { + bi_opt_dead_code_eliminate(ctx); bi_analyze_helper_requirements(ctx); + } /* Fuse TEXC after analyzing helper requirements so the analysis * doesn't have to know about dual textures */