[Mesa-dev] [PATCH 005/133] nir: add core helper functions

[Jason Ekstrand]

On Tue, Dec 16, 2014 at 3:47 PM, Eric Anholt <eric at anholt.net> wrote:
>
> Jason Ekstrand <jason at jlekstrand.net> writes:
>
> > From: Connor Abbott <connor.abbott at intel.com>
> >
> > These include functions for adding and removing various bits of IR and
> > helpers for iterating over all the sources and destinations of an
> > instruction. This is similar to ir.cpp.
>
> > +nir_function_overload *
> > +nir_function_overload_create(nir_function *func)
> > +{
> > +   void *mem_ctx = ralloc_parent(func);
> > +
> > +   nir_function_overload *overload = ralloc(mem_ctx,
> nir_function_overload);
> > +
> > +   overload->num_params = 0;
> > +   overload->params = NULL;
> > +   overload->return_type = glsl_void_type();
> > +
> > +   exec_list_push_tail(&func->overload_list, &overload->node);
> > +   overload->function = func;
> > +
> > +   return overload;
> > +}
>
> Looks like overload->impl is left undefined.
>
> > +nir_tex_instr *
> > +nir_tex_instr_create(void *mem_ctx, unsigned num_srcs)
> > +{
> > +   nir_tex_instr *instr = ralloc(mem_ctx, nir_tex_instr);
> > +   instr_init(&instr->instr, nir_instr_type_texture);
> > +
> > +   instr->num_srcs = num_srcs;
> > +   for (unsigned i = 0; i < num_srcs; i++)
> > +      src_init(&instr->src[i]);
>
> Having to compute the num_srcs up front kind of sucked.  It seems
> important for the ALU ops, but if we're not mallocing less memory based
> on num_srcs, it would be nice to avoid.  Not necessary to change in the
> review process, though.
>
> Also, missing this line that other instruction allocators have:
>    dest_init(&instr->dest);
>
>
> I didn't review the details of CFG manipulation, but other than these
> little comments the rest is:
>

I wouldn't worry too much about the CFG stuff.  Connor was careful when he
wrote it and I've done quite a bit of digging there and have convinced
myself that it's sane.


>
> Reviewed-by: Eric Anholt <eric at anholt.net>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141219/bcb49df6/attachment.html>