[Mesa-dev] [PATCH] nir: remove simple dead if detection from nir_opt_dead_cf()

[Timothy Arceri]

On 15/2/19 4:12 pm, Jason Ekstrand wrote:
> 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.

As far as I understand it all it (and peephole) is really doing is removing:

if (cond) {

} else {

}

Because DCE already removes the dead instructions.

This pass is looking at the instructions in every if it sees. If we 
don't want to depend on peephole we can add a simple check in nir_opt_if 
to look for empty branches. That would be much cheaper.


> 
> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net 
> <mailto:jason at jlekstrand.net>>
> 
> On Wed, Feb 13, 2019 at 7:38 PM Timothy Arceri <tarceri at itsqueeze.com 
> <mailto:tarceri at itsqueeze.com>> wrote:
> 
>     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
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>