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

[Jason Ekstrand]

On Fri, Aug 28, 2015 at 10:49 AM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>
> 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.

Sure, I don't care that much about it.
--Jason

>>
>> >  }
>> >
>> >  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