[Mesa-dev] [PATCH 1/5] nir: Make nir_block_{first, last}_instr return NULL for empty blocks.

[Connor Abbott]

On Aug 27, 2015 11:45 AM, "Jason Ekstrand" <jason at jlekstrand.net> wrote:
>
> On Tue, Aug 25, 2015 at 1:24 PM, Kenneth Graunke <kenneth at whitecape.org>
wrote:
> > This is a lot more reasonable than returning an offset from NULL.
> >
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/glsl/nir/nir.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> > index 40871f7..12ddeb2 100644
> > --- a/src/glsl/nir/nir.h
> > +++ b/src/glsl/nir/nir.h
> > @@ -1219,14 +1219,14 @@ static inline nir_instr *
> >  nir_block_first_instr(nir_block *block)
> >  {
> >     struct exec_node *head = exec_list_get_head(&block->instr_list);
> > -   return exec_node_data(nir_instr, head, node);
> > +   return head ? exec_node_data(nir_instr, head, node) : NULL;
>
> node is the first element of nir_instr so this is a no-op.  Perhaps we
> should instead just add a
>
> STATIC_ASSERT(offsetof(nir_instr, node) == 0)
>
> and a comment.

I don't think that's really necessary, unless we want to really
micro-optimize this function... I'd rather we have the obviously-correct
thing that expresses what we actually want to do (get at the parent struct,
or return NULL if it doesn't exist). We can do this if the compiler isn't
smart enough and it actually does hurt, but given that we're probably
cache-miss limited already, I would very much doubt that the extra few
instructions are worth it.

>
> >  }
> >
> >  static inline nir_instr *
> >  nir_block_last_instr(nir_block *block)
> >  {
> >     struct exec_node *tail = exec_list_get_tail(&block->instr_list);
> > -   return exec_node_data(nir_instr, tail, node);
> > +   return tail ? exec_node_data(nir_instr, tail, node) : NULL;
> >  }
> >
> >  #define nir_foreach_instr(block, instr) \
> > --
> > 2.5.0
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> 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/20150828/a9fef1d3/attachment.html>