From d28b6b6856864f75e82bd5c63b877d1f1a40df9a Mon Sep 17 00:00:00 2001 From: Rhys Perry Date: Wed, 2 Mar 2022 13:32:42 +0000 Subject: [PATCH] aco: rework removal of jumps over branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only allow this in situations where we know it's safe. In particular, this stops removal of unconditional branches like with block_kind_continue_or_break. Fixes dEQP-VK.graphicsfuzz.fragcoord-control-flow hang. fossil-db (Sienna Cichlid): Totals from 34 (0.02% of 162293) affected shaders: Instrs: 84115 -> 84178 (+0.07%); split: -0.00%, +0.08% CodeSize: 463372 -> 463624 (+0.05%); split: -0.00%, +0.06% Latency: 3467316 -> 3467652 (+0.01%) InvThroughput: 3085493 -> 3085578 (+0.00%) Branches: 3221 -> 3284 (+1.96%); split: -0.03%, +1.99% Signed-off-by: Rhys Perry Reviewed-by: Daniel Schürmann Fixes: f030b75b7d2 ("aco: relax condition to remove branches in case of few instructions") Part-of: --- src/amd/compiler/aco_lower_to_hw_instr.cpp | 23 ++++++++++------------ 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/amd/compiler/aco_lower_to_hw_instr.cpp b/src/amd/compiler/aco_lower_to_hw_instr.cpp index 08830bfc57b..d085fb67219 100644 --- a/src/amd/compiler/aco_lower_to_hw_instr.cpp +++ b/src/amd/compiler/aco_lower_to_hw_instr.cpp @@ -2207,7 +2207,6 @@ lower_to_hw_instr(Program* program) bool can_remove = block->index < target; unsigned num_scalar = 0; unsigned num_vector = 0; - bool has_sopp = false; /* Check the instructions between branch and target */ for (unsigned i = block->index + 1; i < branch->target[0]; i++) { @@ -2221,19 +2220,17 @@ lower_to_hw_instr(Program* program) for (aco_ptr& inst : program->blocks[i].instructions) { if (inst->isSOPP()) { - /* we allow at most one inner branch */ - if (has_sopp) + /* Discard early exits and loop breaks and continues should work fine with an + * empty exec mask. + */ + bool is_break_continue = + program->blocks[i].kind & (block_kind_break | block_kind_continue); + bool discard_early_exit = + discard_block && (unsigned)inst->sopp().block == discard_block->index; + if ((inst->opcode != aco_opcode::s_cbranch_scc0 && + inst->opcode != aco_opcode::s_cbranch_scc1) || + (!discard_early_exit && !is_break_continue)) can_remove = false; - - /* These instructions must conditionally be jumped over */ - if (inst->opcode == aco_opcode::s_endpgm || - inst->opcode == aco_opcode::s_sendmsg || - inst->opcode == aco_opcode::s_sendmsghalt || - inst->opcode == aco_opcode::s_trap || - inst->opcode == aco_opcode::s_barrier) - can_remove = false; - - has_sopp = true; } else if (inst->isSALU()) { num_scalar++; } else if (inst->isVALU() || inst->isVINTRP()) {