From 8571c577aa02be048826f8667b93709c3c620fc5 Mon Sep 17 00:00:00 2001 From: Caio Marcelo de Oliveira Filho Date: Mon, 19 Mar 2018 16:34:17 -0700 Subject: nir/dead_cf: also remove useless ifs Generalize the code for remove dead loops to also remove dead if nodes. The conditions are the same in both cases, if the node (and it's children) don't have side-effects AND the nodes after it don't use the values produced by the node. The only difference is when evaluating side effects: loops consider only return jumps as a side-effect -- they can stop execution of nodes after it; 'if' nodes outside loops should consider all kinds of jumps (return, break, continue) since all of them can cause execution of nodes after it to be skipped. After this patch, empty ifs (those which both then and else blocks are empty) will be removed by nir_opt_dead_cf. It caused no change to shader-db, in part because the removal of empty ifs is currently covered by nir_opt_peephole_select. v2: Improve the identification of cases where break/continue can cause side-effects. (Jason) v3: Move code comment changes to a different patch. (Jason) Reviewed-by: Jason Ekstrand --- src/compiler/nir/nir_opt_dead_cf.c | 44 ++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 14 deletions(-) (limited to 'src/compiler') diff --git a/src/compiler/nir/nir_opt_dead_cf.c b/src/compiler/nir/nir_opt_dead_cf.c index b15dfdd0c91..a652bcd99bb 100644 --- a/src/compiler/nir/nir_opt_dead_cf.c +++ b/src/compiler/nir/nir_opt_dead_cf.c @@ -60,12 +60,12 @@ * } * ... * - * Finally, we also handle removing useless loops, i.e. loops with no side - * effects and without any definitions that are used elsewhere. This case is a - * little different from the first two in that the code is actually run (it - * just never does anything), but there are similar issues with needing to - * be careful with restarting after deleting the cf_node (see dead_cf_list()) - * so this is a convenient place to remove them. + * Finally, we also handle removing useless loops and ifs, i.e. loops and ifs + * with no side effects and without any definitions that are used + * elsewhere. This case is a little different from the first two in that the + * code is actually run (it just never does anything), but there are similar + * issues with needing to be careful with restarting after deleting the + * cf_node (see dead_cf_list()) so this is a convenient place to remove them. */ static void @@ -136,6 +136,12 @@ static bool cf_node_has_side_effects(nir_cf_node *node) { nir_foreach_block_in_cf_node(block, node) { + bool inside_loop = node->type == nir_cf_node_loop; + for (nir_cf_node *n = &block->cf_node; !inside_loop && n != node; n = n->parent) { + if (n->type == nir_cf_node_loop) + inside_loop = true; + } + nir_foreach_instr(instr, block) { if (instr->type == nir_instr_type_call) return true; @@ -143,10 +149,13 @@ cf_node_has_side_effects(nir_cf_node *node) /* Return instructions can cause us to skip over other side-effecting * instructions after the loop, so consider them to have side effects * here. + * + * When the block is not inside a loop, break and continue might also + * cause a skip. */ if (instr->type == nir_instr_type_jump && - nir_instr_as_jump(instr)->type == nir_jump_return) + (!inside_loop || nir_instr_as_jump(instr)->type == nir_jump_return)) return true; if (instr->type != nir_instr_type_intrinsic) @@ -171,7 +180,7 @@ def_not_live_out(nir_ssa_def *def, void *state) } /* - * Test if a loop is dead. A loop node is dead if: + * Test if a loop node or if 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 @@ -187,19 +196,21 @@ def_not_live_out(nir_ssa_def *def, void *state) */ static bool -loop_is_dead(nir_loop *loop) +node_is_dead(nir_cf_node *node) { - nir_block *before = nir_cf_node_as_block(nir_cf_node_prev(&loop->cf_node)); - nir_block *after = nir_cf_node_as_block(nir_cf_node_next(&loop->cf_node)); + assert(node->type == nir_cf_node_loop || node->type == nir_cf_node_if); + + 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)); if (!exec_list_is_empty(&after->instr_list) && nir_block_first_instr(after)->type == nir_instr_type_phi) return false; - if (cf_node_has_side_effects(&loop->cf_node)) + if (cf_node_has_side_effects(node)) return false; - nir_function_impl *impl = nir_cf_node_get_function(&loop->cf_node); + nir_function_impl *impl = nir_cf_node_get_function(node); nir_metadata_require(impl, nir_metadata_live_ssa_defs | nir_metadata_dominance); @@ -219,6 +230,11 @@ 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; + } + nir_const_value *const_value = nir_src_as_const_value(following_if->condition); @@ -233,7 +249,7 @@ dead_cf_block(nir_block *block) if (!following_loop) return false; - if (!loop_is_dead(following_loop)) + if (!node_is_dead(&following_loop->cf_node)) return false; nir_cf_node_remove(&following_loop->cf_node); -- cgit v1.2.3