[Mesa-dev] [PATCH v2 02/14] nir: insert ssa_undef instructions when cleaning up defs/uses

[Jason Ekstrand]

On May 21, 2015 9:41 AM, "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

"unreachable instructions" plural

> 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.
>
> v2: split out other fixes
> Signed-off-by: Connor Abbott <cwabbott0 at gmail.com>
> ---
>  src/glsl/nir/nir.c | 38 ++++++++++++++++++++++++++++----------
>  1 file changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
> index f03e80a..704553f 100644
> --- a/src/glsl/nir/nir.c
> +++ b/src/glsl/nir/nir.c
> @@ -1206,26 +1206,26 @@ 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);
> +         remove_defs_uses(instr, impl);
>        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 +1234,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:
> @@ -1443,16 +1442,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);

I'm not entirely happy that this is done unconditionally here given that it
may be somewhat expensive and most of the time isn't needed.  However, I'm
not going to quibble over it too bad.

Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

> +   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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150521/c583bd66/attachment.html>