[Mesa-dev] [PATCH 2/9] nir: insert ssa_undef instructions when cleanup up defs/uses

[Jason Ekstrand]

On Fri, May 8, 2015 at 10:03 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
> The point of cleanup_defs_uses() is to make an instruction safe to
> remove by removing any references that the rest of the shader may have
> to it. Previously, it was handling register use/def sets and removing
> the instruction from the use sets of any SSA sources it had, but if the
> instruction defined an SSA value that was used by other instructions it
> wasn't doing anything. This was ok, since we were always careful to make
> sure that no removed instruction ever had any uses, but now we want to
> start removing unreachable instruction which might still be used in
> reachable parts of the code. In that case, the value that any uses get
> is undefined since the instruction never actually executes, so we can
> just replace the instruction with an ssa_undef_instr.
>
> Signed-off-by: Connor Abbott <cwabbott0 at gmail.com>
> ---
>  src/glsl/nir/nir.c | 47 ++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 34 insertions(+), 13 deletions(-)
>
> diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
> index f03e80a..362ac30 100644
> --- a/src/glsl/nir/nir.c
> +++ b/src/glsl/nir/nir.c
> @@ -1206,26 +1206,29 @@ stitch_blocks(nir_block *before, nir_block *after)
>  }
>
>  static void
> -remove_defs_uses(nir_instr *instr);
> +remove_defs_uses(nir_instr *instr, nir_function_impl *impl);
>
>  static void
> -cleanup_cf_node(nir_cf_node *node)
> +cleanup_cf_node(nir_cf_node *node, nir_function_impl *impl)
>  {
>     switch (node->type) {
>     case nir_cf_node_block: {
>        nir_block *block = nir_cf_node_as_block(node);
>        /* We need to walk the instructions and clean up defs/uses */
> -      nir_foreach_instr(block, instr)
> -         remove_defs_uses(instr);
> +      nir_foreach_instr(block, instr) {
> +         if (instr->type == nir_instr_type_jump)
> +            handle_remove_jump(block, nir_instr_as_jump(instr)->type);
> +         remove_defs_uses(instr, impl);
> +      }

This looks like an unrelated change.  Maybe split that out?  The rest
of the patch looks plausible.
--Jason

>        break;
>     }
>
>     case nir_cf_node_if: {
>        nir_if *if_stmt = nir_cf_node_as_if(node);
>        foreach_list_typed(nir_cf_node, child, node, &if_stmt->then_list)
> -         cleanup_cf_node(child);
> +         cleanup_cf_node(child, impl);
>        foreach_list_typed(nir_cf_node, child, node, &if_stmt->else_list)
> -         cleanup_cf_node(child);
> +         cleanup_cf_node(child, impl);
>
>        list_del(&if_stmt->condition.use_link);
>        break;
> @@ -1234,13 +1237,12 @@ cleanup_cf_node(nir_cf_node *node)
>     case nir_cf_node_loop: {
>        nir_loop *loop = nir_cf_node_as_loop(node);
>        foreach_list_typed(nir_cf_node, child, node, &loop->body)
> -         cleanup_cf_node(child);
> +         cleanup_cf_node(child, impl);
>        break;
>     }
>     case nir_cf_node_function: {
> -      nir_function_impl *impl = nir_cf_node_as_function(node);
>        foreach_list_typed(nir_cf_node, child, node, &impl->body)
> -         cleanup_cf_node(child);
> +         cleanup_cf_node(child, impl);
>        break;
>     }
>     default:
> @@ -1254,6 +1256,8 @@ nir_cf_node_remove(nir_cf_node *node)
>     nir_function_impl *impl = nir_cf_node_get_function(node);
>     nir_metadata_preserve(impl, nir_metadata_none);
>
> +   cleanup_cf_node(node, impl);
> +
>     if (node->type == nir_cf_node_block) {
>        /*
>         * Basic blocks can't really be removed by themselves, since they act as
> @@ -1275,8 +1279,6 @@ nir_cf_node_remove(nir_cf_node *node)
>        exec_node_remove(&node->node);
>        stitch_blocks(before_block, after_block);
>     }
> -
> -   cleanup_cf_node(node);
>  }
>
>  static bool
> @@ -1443,16 +1445,35 @@ remove_def_cb(nir_dest *dest, void *state)
>     return true;
>  }
>
> +static bool
> +remove_ssa_def_cb(nir_ssa_def *def, void *state)
> +{
> +   nir_function_impl *impl = state;
> +   nir_shader *shader = impl->overload->function->shader;
> +
> +   if (!list_empty(&def->uses) || !list_empty(&def->if_uses)) {
> +      nir_ssa_undef_instr *undef =
> +         nir_ssa_undef_instr_create(shader, def->num_components);
> +      nir_instr_insert_before_cf_list(&impl->body, &undef->instr);
> +      nir_ssa_def_rewrite_uses(def, nir_src_for_ssa(&undef->def), shader);
> +   }
> +
> +   return true;
> +}
> +
> +
>  static void
> -remove_defs_uses(nir_instr *instr)
> +remove_defs_uses(nir_instr *instr, nir_function_impl *impl)
>  {
>     nir_foreach_dest(instr, remove_def_cb, instr);
>     nir_foreach_src(instr, remove_use_cb, instr);
> +   nir_foreach_ssa_def(instr, remove_ssa_def_cb, impl);
>  }
>
>  void nir_instr_remove(nir_instr *instr)
>  {
> -   remove_defs_uses(instr);
> +   nir_function_impl *impl = nir_cf_node_get_function(&instr->block->cf_node);
> +   remove_defs_uses(instr, impl);
>     exec_node_remove(&instr->node);
>
>     if (instr->type == nir_instr_type_jump) {
> --
> 2.1.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev