From 77c1c688dc962c51df145e0faa6d290e30a0ad8e Mon Sep 17 00:00:00 2001 From: Job Noorman Date: Tue, 19 Aug 2025 15:30:23 +0200 Subject: [PATCH] ir3/array_to_ssa: remove trivial all-undef phis remove_trivial_phi erroneously skipped phis containing an undef src because the remaining srcs may not dominate the phi. However, it's fine to replace a phi whose srcs are all undef with undef. Fix this by simply checking if all srcs are equal, whether undef or not. Note that in practice, this often caused phis with undef srcs to be inserted all the way up to the entry block, keeping their defs alive for much longer than necessary. Fixes unnecessary spilling in God Of War and Neon Noir traces. Totals: MaxWaves: 2381774 -> 2384954 (+0.13%) Instrs: 49052269 -> 49052865 (+0.00%); split: -0.03%, +0.04% CodeSize: 102493810 -> 102514296 (+0.02%); split: -0.02%, +0.04% NOPs: 8391570 -> 8385296 (-0.07%); split: -0.14%, +0.07% MOVs: 1448918 -> 1455153 (+0.43%); split: -0.43%, +0.86% COVs: 824835 -> 824846 (+0.00%) Full: 1714015 -> 1707987 (-0.35%) (ss): 1125974 -> 1126692 (+0.06%); split: -0.14%, +0.21% (sy): 553893 -> 553561 (-0.06%); split: -0.23%, +0.17% (ss)-stall: 4011440 -> 4006144 (-0.13%); split: -0.21%, +0.08% (sy)-stall: 16707741 -> 16664838 (-0.26%); split: -0.48%, +0.23% STPs: 18953 -> 18495 (-2.42%) LDPs: 23957 -> 22121 (-7.66%) Preamble Instrs: 11100893 -> 11100673 (-0.00%) Early Preamble: 122185 -> 122188 (+0.00%) Last helper: 11913048 -> 11914963 (+0.02%); split: -0.04%, +0.06% Subgroup size: 12925248 -> 12926272 (+0.01%) Cat0: 9246551 -> 9240417 (-0.07%); split: -0.13%, +0.07% Cat1: 2335781 -> 2341487 (+0.24%); split: -0.29%, +0.53% Cat2: 18445905 -> 18445930 (+0.00%) Cat6: 515382 -> 514732 (-0.13%) Cat7: 1635575 -> 1637224 (+0.10%); split: -0.09%, +0.19% Totals from 2293 (1.39% of 164705) affected shaders: MaxWaves: 21622 -> 24802 (+14.71%) Instrs: 3399456 -> 3400052 (+0.02%); split: -0.49%, +0.51% CodeSize: 6576806 -> 6597292 (+0.31%); split: -0.24%, +0.55% NOPs: 774365 -> 768091 (-0.81%); split: -1.54%, +0.73% MOVs: 226724 -> 232959 (+2.75%); split: -2.73%, +5.48% COVs: 48005 -> 48016 (+0.02%) Full: 50599 -> 44571 (-11.91%) (ss): 88248 -> 88966 (+0.81%); split: -1.85%, +2.66% (sy): 41345 -> 41013 (-0.80%); split: -3.03%, +2.23% (ss)-stall: 396793 -> 391497 (-1.33%); split: -2.11%, +0.78% (sy)-stall: 1594786 -> 1551883 (-2.69%); split: -5.06%, +2.37% STPs: 1147 -> 689 (-39.93%) LDPs: 2535 -> 699 (-72.43%) Preamble Instrs: 707407 -> 707187 (-0.03%) Early Preamble: 180 -> 183 (+1.67%) Last helper: 1538341 -> 1540256 (+0.12%); split: -0.35%, +0.47% Subgroup size: 149248 -> 150272 (+0.69%) Cat0: 857696 -> 851562 (-0.72%); split: -1.43%, +0.72% Cat1: 275565 -> 281271 (+2.07%); split: -2.44%, +4.51% Cat2: 1139467 -> 1139492 (+0.00%) Cat6: 22505 -> 21855 (-2.89%) Cat7: 129600 -> 131249 (+1.27%); split: -1.15%, +2.42% Signed-off-by: Job Noorman Part-of: --- src/freedreno/ir3/ir3_array_to_ssa.c | 38 +++++++++++----------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/src/freedreno/ir3/ir3_array_to_ssa.c b/src/freedreno/ir3/ir3_array_to_ssa.c index 2b3849f8255..2e9d6820663 100644 --- a/src/freedreno/ir3/ir3_array_to_ssa.c +++ b/src/freedreno/ir3/ir3_array_to_ssa.c @@ -121,40 +121,32 @@ remove_trivial_phi(struct ir3_instruction *phi) phi->data = phi->dsts[0]; struct ir3_register *unique_def = NULL; + bool unique_def_valid = false; bool unique = true; for (unsigned i = 0; i < phi->block->predecessors_count; i++) { struct ir3_register *src = phi->srcs[i]; - /* If there are any undef sources, then the remaining sources may not - * dominate the phi node, even if they are all equal. So we need to - * bail out in this case. - * - * This seems to be a bug in the original paper. - */ - if (!src->def) { - unique = false; - break; + if (src->def) { + struct ir3_instruction *src_instr = src->def->instr; + + /* phi sources which point to the phi itself don't count for + * figuring out if the phi is trivial + */ + if (src_instr == phi) + continue; + + if (src_instr->opc == OPC_META_PHI) { + src->def = remove_trivial_phi(src->def->instr); + } } - struct ir3_instruction *src_instr = src->def->instr; - - /* phi sources which point to the phi itself don't count for - * figuring out if the phi is trivial - */ - if (src_instr == phi) - continue; - - if (src_instr->opc == OPC_META_PHI) { - src->def = remove_trivial_phi(src->def->instr); - } - - if (unique_def) { + if (unique_def_valid) { if (unique_def != src->def) { unique = false; - break; } } else { unique_def = src->def; + unique_def_valid = true; } }