<div dir="ltr"><div>I think the primary issue we had with dead ifs before is that we were running dce and dead_cf but maybe not peephole_select because it didn't seem applicable. In principle, I think I like dead_cf being able to handle if statements as well because it seems like a thing it should do. However, it's obviously very expensive so I'm happy for us to drop it for now. One day, maybe we can make the pass less expensive so it doesn't walk the list of instructions nearly as many times and making it handle ifs will be more practical. For now, however, I think this is the right thing to do. </div><div> </div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>></div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Feb 13, 2019 at 7:38 PM Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com">tarceri@itsqueeze.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This was probably useful when it was first written, however it looks to be no longer necessary. As far as I can tell these days dce is smart enough to remove useless instructions from if branches. Once this is done nir_opt_peephole_select() will end up removing the empty if. Removing this support reduces the dolphin uber shader compilation time by around 60%. Compile time is reduced due to no longer having to compute the live ssa defs metadata so much. No shader-db changes on i965 or radeonsi. --- src/compiler/nir/nir_opt_dead_cf.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/compiler/nir/nir_opt_dead_cf.c b/src/compiler/nir/nir_opt_dead_cf.c index 14986732096..053c5743527 100644 --- a/src/compiler/nir/nir_opt_dead_cf.c +++ b/src/compiler/nir/nir_opt_dead_cf.c @@ -180,7 +180,7 @@ def_not_live_out(nir_ssa_def *def, void *state) } /* - * Test if a loop node or if node is dead. Such nodes are dead if: + * Test if a loop node is dead. Such nodes are dead if: * * 1) It has no side effects (i.e. intrinsics which could possibly affect the * state of the program aside from producing an SSA value, indicated by a lack @@ -198,7 +198,7 @@ def_not_live_out(nir_ssa_def *def, void *state) static bool node_is_dead(nir_cf_node *node) { - assert(node->type == nir_cf_node_loop || node->type == nir_cf_node_if); + assert(node->type == nir_cf_node_loop); nir_block *before = nir_cf_node_as_block(nir_cf_node_prev(node)); nir_block *after = nir_cf_node_as_block(nir_cf_node_next(node)); @@ -230,11 +230,6 @@ dead_cf_block(nir_block *block) { nir_if *following_if = nir_block_get_following_if(block); if (following_if) { - if (node_is_dead(&following_if->cf_node)) { - nir_cf_node_remove(&following_if->cf_node); - return true; - } - if (!nir_src_is_const(following_if->condition)) return false; -- 2.20.1 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a></blockquote></div>