On Fri, May 8, 2015 at 10:03 PM, Connor Abbott <cwabbo...@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 <cwabbo...@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
