On Tue, Dec 16, 2014 at 3:47 PM, Eric Anholt <e...@anholt.net> wrote:
>
> Jason Ekstrand <ja...@jlekstrand.net> writes:
>
> > From: Connor Abbott <connor.abb...@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 <e...@anholt.net>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
