<div dir="ltr"><div>Both are </div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Mar 19, 2018 at 4:34 PM, Caio Marcelo de Oliveira Filho <<a href="mailto:caio.oliveira@intel.com" target="_blank">caio.oliveira@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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) --- src/compiler/nir/nir_opt_dead_cf.c | 44 ++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/src/compiler/nir/nir_opt_dead_cf.c b/src/compiler/nir/nir_opt_dead_cf.c index b15dfdd0c9..a652bcd99b 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) <div class="HOEnZb"><div class="h5"> 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); -- 2.16.2 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">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> </div></div></blockquote></div> </div>