From 60776f87c38f69507d60591b46b3ea2efba8e188 Mon Sep 17 00:00:00 2001 From: Georg Lehmann Date: Wed, 4 Sep 2024 18:33:59 +0200 Subject: [PATCH] nir/opt_remove_phis: rematerialize constants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Foz-DB Navi31: Totals from 749 (0.94% of 79395) affected shaders: Instrs: 1224359 -> 1223722 (-0.05%); split: -0.07%, +0.02% CodeSize: 6468392 -> 6466296 (-0.03%); split: -0.06%, +0.03% Latency: 9764410 -> 9766457 (+0.02%); split: -0.01%, +0.03% InvThroughput: 1017401 -> 1017380 (-0.00%); split: -0.03%, +0.03% VClause: 19902 -> 19873 (-0.15%); split: -0.16%, +0.02% SClause: 38441 -> 38424 (-0.04%); split: -0.05%, +0.01% Copies: 86880 -> 86304 (-0.66%); split: -0.73%, +0.06% Branches: 34206 -> 34159 (-0.14%); split: -0.14%, +0.01% PreSGPRs: 45557 -> 45527 (-0.07%); split: -0.08%, +0.01% PreVGPRs: 32406 -> 32408 (+0.01%) VALU: 671633 -> 671533 (-0.01%); split: -0.02%, +0.01% SALU: 155284 -> 154675 (-0.39%); split: -0.40%, +0.00% VMEM: 27303 -> 27271 (-0.12%) SMEM: 67490 -> 67455 (-0.05%) Reviewed-by: Daniel Schürmann Part-of: --- src/compiler/nir/nir_opt_remove_phis.c | 91 ++++++++++++++++---------- 1 file changed, 56 insertions(+), 35 deletions(-) diff --git a/src/compiler/nir/nir_opt_remove_phis.c b/src/compiler/nir/nir_opt_remove_phis.c index bf467bf116c..22c0f57a69f 100644 --- a/src/compiler/nir/nir_opt_remove_phis.c +++ b/src/compiler/nir/nir_opt_remove_phis.c @@ -28,24 +28,48 @@ #include "nir.h" #include "nir_builder.h" -static nir_alu_instr * -get_parent_mov(nir_def *ssa) +static bool +phi_srcs_equal(nir_def *a, nir_def *b) { - if (ssa->parent_instr->type != nir_instr_type_alu) - return NULL; + if (a == b) + return true; - nir_alu_instr *alu = nir_instr_as_alu(ssa->parent_instr); - return (alu->op == nir_op_mov) ? alu : NULL; + if (a->parent_instr->type != b->parent_instr->type) + return false; + + if (a->parent_instr->type != nir_instr_type_alu && + a->parent_instr->type != nir_instr_type_load_const) + return false; + + if (!nir_instrs_equal(a->parent_instr, b->parent_instr)) + return false; + + /* nir_instrs_equal ignores exact/fast_math */ + if (a->parent_instr->type == nir_instr_type_alu) { + nir_alu_instr *a_alu = nir_instr_as_alu(a->parent_instr); + nir_alu_instr *b_alu = nir_instr_as_alu(b->parent_instr); + if (a_alu->exact != b_alu->exact || a_alu->fp_fast_math != b_alu->fp_fast_math) + return false; + } + + return true; } static bool -matching_mov(nir_alu_instr *mov1, nir_def *ssa) +can_rematerialize_phi_src(nir_block *block, nir_def *def) { - if (!mov1) - return false; - - nir_alu_instr *mov2 = get_parent_mov(ssa); - return mov2 && nir_alu_srcs_equal(mov1, mov2, 0, 0); + if (def->parent_instr->type == nir_instr_type_alu) { + /* Restrict alu to movs. */ + nir_alu_instr *alu = nir_instr_as_alu(def->parent_instr); + if (alu->op != nir_op_mov) + return false; + if (!nir_block_dominates(alu->src[0].src.ssa->parent_instr->block, block->imm_dom)) + return false; + return true; + } else if (def->parent_instr->type == nir_instr_type_load_const) { + return true; + } + return false; } /* @@ -70,8 +94,8 @@ remove_phis_block(nir_block *block, nir_builder *b) nir_foreach_phi_safe(phi, block) { nir_def *def = NULL; - nir_alu_instr *mov = NULL; bool srcs_same = true; + bool needs_remat = false; nir_foreach_phi_src(src, phi) { /* For phi nodes at the beginning of loops, we may encounter some @@ -88,17 +112,22 @@ remove_phis_block(nir_block *block, nir_builder *b) if (src->src.ssa == &phi->def) continue; + /* Ignore undef sources. */ + if (nir_src_is_undef(src->src)) + continue; + if (def == NULL) { def = src->src.ssa; - mov = get_parent_mov(def); - } else if (nir_src_is_undef(src->src) && - nir_block_dominates(def->parent_instr->block, src->pred)) { - /* Ignore this undef source. */ - } else { - if (src->src.ssa != def && !matching_mov(mov, src->src.ssa)) { - srcs_same = false; - break; + if (!nir_block_dominates(def->parent_instr->block, block->imm_dom)) { + if (!can_rematerialize_phi_src(block, def)) { + srcs_same = false; + break; + } + needs_remat = true; } + } else if (!phi_srcs_equal(src->src.ssa, def)) { + srcs_same = false; + break; } } @@ -106,22 +135,14 @@ remove_phis_block(nir_block *block, nir_builder *b) continue; if (!def) { - /* In this case, the phi had no sources. So turn it into an undef. */ - + /* In this case, the phi had no non undef sources. So turn it into an undef. */ b->cursor = nir_after_phis(block); - def = nir_undef(b, phi->def.num_components, - phi->def.bit_size); - } else if (mov) { - /* If the sources were all movs from the same source with the same - * swizzle, then we can't just pick a random move because it may not - * dominate the phi node. Instead, we need to emit our own move after - * the phi which uses the shared source, and rewrite uses of the phi - * to use the move instead. This is ok, because while the movs may - * not all dominate the phi node, their shared source does. - */ - + def = nir_undef(b, phi->def.num_components, phi->def.bit_size); + } else if (needs_remat) { b->cursor = nir_after_phis(block); - def = nir_mov_alu(b, mov->src[0], def->num_components); + nir_instr *remat = nir_instr_clone(b->shader, def->parent_instr); + nir_builder_instr_insert(b, remat); + def = nir_instr_def(remat); } nir_def_replace(&phi->def, def);