<p dir="ltr"><br>
On Aug 27, 2015 11:45 AM, "Jason Ekstrand" <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
><br>
> On Tue, Aug 25, 2015 at 1:24 PM, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote:<br>
> > This is a lot more reasonable than returning an offset from NULL.<br>
> ><br>
> > Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>><br>
> > ---<br>
> >  src/glsl/nir/nir.h | 4 ++--<br>
> >  1 file changed, 2 insertions(+), 2 deletions(-)<br>
> ><br>
> > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h<br>
> > index 40871f7..12ddeb2 100644<br>
> > --- a/src/glsl/nir/nir.h<br>
> > +++ b/src/glsl/nir/nir.h<br>
> > @@ -1219,14 +1219,14 @@ static inline nir_instr *<br>
> >  nir_block_first_instr(nir_block *block)<br>
> >  {<br>
> >     struct exec_node *head = exec_list_get_head(&block->instr_list);<br>
> > -   return exec_node_data(nir_instr, head, node);<br>
> > +   return head ? exec_node_data(nir_instr, head, node) : NULL;<br>
><br>
> node is the first element of nir_instr so this is a no-op.  Perhaps we<br>
> should instead just add a<br>
><br>
> STATIC_ASSERT(offsetof(nir_instr, node) == 0)<br>
><br>
> and a comment.</p>
<p dir="ltr">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.</p>
<p dir="ltr">><br>
> >  }<br>
> ><br>
> >  static inline nir_instr *<br>
> >  nir_block_last_instr(nir_block *block)<br>
> >  {<br>
> >     struct exec_node *tail = exec_list_get_tail(&block->instr_list);<br>
> > -   return exec_node_data(nir_instr, tail, node);<br>
> > +   return tail ? exec_node_data(nir_instr, tail, node) : NULL;<br>
> >  }<br>
> ><br>
> >  #define nir_foreach_instr(block, instr) \<br>
> > --<br>
> > 2.5.0<br>
> ><br>
> > _______________________________________________<br>
> > mesa-dev mailing list<br>
> > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
> _______________________________________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</p>