From 855e29cd78f2870a0937a6b105d4bc6aafd9cb44 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Mon, 25 Jan 2021 12:08:15 +0100 Subject: [PATCH] pan/bi: Make sure we never branch to an non-existing clause Branch instructions can point to the last shader block, which might be empty. In this case, the branch points to a clause that doesn't exists, leading to INVALID_ENC faults when the GPU tries to jump to this clause. Check if the block is a terminal block before updating the branch offset: jumping/branching to NULL is equivalent to a shader termination, so the default value of 0 works just fine for terminal branches. Signed-off-by: Boris Brezillon Reviewed-by: Alyssa Rosenzweig Part-of: --- src/panfrost/bifrost/bi_pack.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/panfrost/bifrost/bi_pack.c b/src/panfrost/bifrost/bi_pack.c index ead30c50e64..95a0ef1624f 100644 --- a/src/panfrost/bifrost/bi_pack.c +++ b/src/panfrost/bifrost/bi_pack.c @@ -561,23 +561,28 @@ bi_pack_constants(bi_context *ctx, bi_clause *clause, }; /* Compute branch offset instead of a dummy 0 */ + bool terminal_branch = true; + if (branches) { bi_instr *br = clause->tuples[clause->tuple_count - 1].add; assert(br && br->branch_target); - /* Put it in the high place */ - int32_t qwords = bi_block_offset(ctx, clause, br->branch_target); - int32_t bytes = qwords * 16; + if (!bi_is_terminal_block(ctx, br->branch_target)) { + /* Put it in the high place */ + int32_t qwords = bi_block_offset(ctx, clause, br->branch_target); + int32_t bytes = qwords * 16; - /* Copy so we get proper sign behaviour */ - uint32_t raw = 0; - memcpy(&raw, &bytes, sizeof(raw)); + /* Copy so we get proper sign behaviour */ + uint32_t raw = 0; + memcpy(&raw, &bytes, sizeof(raw)); - /* Clear off top bits for the magic bits */ - raw &= ~0xF0000000; + /* Clear off top bits for the magic bits */ + raw &= ~0xF0000000; + terminal_branch = false; - /* Put in top 32-bits */ - clause->constants[index + 0] = ((uint64_t) raw) << 32ull; + /* Put in top 32-bits */ + clause->constants[index + 0] = ((uint64_t) raw) << 32ull; + } } uint64_t hi = clause->constants[index + 0] >> 60ull; @@ -589,7 +594,7 @@ bi_pack_constants(bi_context *ctx, bi_clause *clause, .imm_2 = ((hi < 8) ? (hi << 60ull) : 0) >> 4, }; - if (branches) { + if (branches && !terminal_branch) { /* Branch offsets are less than 60-bits so this should work at * least for now */ quad.imm_1 |= (4ull << 60ull) >> 4;