[Mesa-dev] [PATCH 02/10] nir: Properly clean up CF nodes when we remove them

[Jason Ekstrand]

On Thu, Feb 5, 2015 at 3:27 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:

> While we're cleaning this up, we should go a little farther - we
> should also see if anything is using the SSA result of any of these
> instructions, and if so make it point to an undef_instr instead to
> avoid dangling pointers. This doesn't come up yet but it will when we
> start cleaning up constant control flow.
>

I'm about to send a v2 and I'm leaving this out for now.  First, because
that's a bit more invasive than I want to do/test right now and second,
because I'm not convinced it's needed.  For control flow, what we really
need to do is assert that there are no phi nodes.  As long as that and the
dominance property both hold, we won't have any uses of deleted SSA values.


> On Thu, Feb 5, 2015 at 5:28 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> > Previously, if you remved a CF node that still had instructions in it,
> none
> > of the use/def information from those instructions would get cleaned up.
> > Also, we weren't removing if statements from the if_uses of the
> > corresponding register or SSA def.  This commit fixes both of these
> > problems
> > ---
> >  src/glsl/nir/nir.c | 54
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 54 insertions(+)
> >
> > diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
> > index 9a88bd3..8ea7bb5 100644
> > --- a/src/glsl/nir/nir.c
> > +++ b/src/glsl/nir/nir.c
> > @@ -1148,6 +1148,58 @@ stitch_blocks(nir_block *before, nir_block *after)
> >     exec_node_remove(&after->cf_node.node);
> >  }
> >
> > +static void
> > +remove_defs_uses(nir_instr *instr);
> > +
> > +static void
> > +cleanup_cf_node(nir_cf_node *node)
> > +{
> > +   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);
> > +      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);
> > +      foreach_list_typed(nir_cf_node, child, node, &if_stmt->else_list)
> > +         cleanup_cf_node(child);
> > +
> > +      struct set *if_uses;
> > +      if (if_stmt->condition.is_ssa) {
> > +         if_uses = if_stmt->condition.ssa->if_uses;
> > +      } else {
> > +         if_uses = if_stmt->condition.reg.reg->if_uses;
> > +      }
> > +
> > +      struct set_entry *entry = _mesa_set_search(if_uses, if_stmt);
> > +      assert(entry);
> > +      _mesa_set_remove(if_uses, entry);
> > +      break;
> > +   }
> > +
> > +   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);
> > +      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);
> > +      break;
> > +   }
> > +   default:
> > +      unreachable("Invalid CF node type");
> > +   }
> > +}
> > +
> >  void
> >  nir_cf_node_remove(nir_cf_node *node)
> >  {
> > @@ -1175,6 +1227,8 @@ 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
> > --
> > 2.2.2
> >
> > _______________________________________________
> > 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/20150209/8d141936/attachment-0001.html>