From 31b5f5a51f3a3d19600dd43bf6ab49bab98a9bbe Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 30 Aug 2023 18:10:28 -0400 Subject: [PATCH] nir/opt_if: Simplify if's with general conditions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dolphin ubershaders have a pattern: if (x && y) { } else { discard; } The current code to simplify if's will bail on this pattern, since the condition is not a comparison. However, if that check is dropped and we allow NIR to invert this, we get: if (!(x && y)) { discard; } else { } which is now in a form for nir_opt_conditional_discard to turn into it discard_if(!(x && y)) which may be substantially cheaper than the original code. In general, I see no reason to restrict to conditionals. Assuming the backend is clever enough to delete empty else blocks (I think most are), then this patch is a strict win as long as inot instructions are cheaper than empty else blocks. This matches my intuition for typical GPUs, where simple ALU instructions are cheaper than control flow. Furthermore, it may be possible in practice for backends to fold the inot into a richer set of instructions. For example, most GPUs have a NAND instructions which would fold in the inot in the above code. So just drop the check, simplify the pass, get the win. --- Also, to avoid inflating register pressure, make sure we put the inot right before the if. Android shader-db on is uninspiring due to terrible coalescing decisions in the current RA. But it does fix the Dolphin smell. total instructions in shared programs: 1756571 -> 1756568 (<.01%) instructions in affected programs: 1600 -> 1597 (-0.19%) helped: 1 HURT: 4 Inconclusive result (value mean confidence interval includes 0). total bytes in shared programs: 11521172 -> 11521156 (<.01%) bytes in affected programs: 10080 -> 10064 (-0.16%) helped: 1 HURT: 4 Inconclusive result (value mean confidence interval includes 0). Signed-off-by: Alyssa Rosenzweig Reviewed-by: Daniel Schürmann Part-of: --- src/compiler/nir/nir_opt_if.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index 1215f962616..6b2bb120f3c 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -861,22 +861,9 @@ opt_if_simplification(nir_builder *b, nir_if *nif) is_block_empty(nir_if_first_else_block(nif))) return false; - /* Make sure the condition is a comparison operation. */ - nir_instr *src_instr = nif->condition.ssa->parent_instr; - if (src_instr->type != nir_instr_type_alu) - return false; - - nir_alu_instr *alu_instr = nir_instr_as_alu(src_instr); - if (!nir_alu_instr_is_comparison(alu_instr)) - return false; - /* Insert the inverted instruction and rewrite the condition. */ - b->cursor = nir_after_instr(&alu_instr->instr); - - nir_def *new_condition = - nir_inot(b, &alu_instr->def); - - nir_src_rewrite(&nif->condition, new_condition); + b->cursor = nir_before_src(&nif->condition); + nir_src_rewrite(&nif->condition, nir_inot(b, nif->condition.ssa)); /* Grab pointers to the last then/else blocks for fixing up the phis. */ nir_block *then_block = nir_if_last_then_block(nif);