[Mesa-dev] [PATCH v2 05/11] nir: Add a structure splitting pass

[Jason Ekstrand]

On Mon, Aug 20, 2018 at 6:41 PM Caio Marcelo de Oliveira Filho <
caio.oliveira at intel.com> wrote:

> On Sat, Jul 28, 2018 at 10:44:36PM -0700, Jason Ekstrand wrote:
> > This pass doesn't really do much now because nir_lower_vars_to_ssa can
> > already see through structures and considers them to be "split".  This
> > pass exists to help other passes more easily see through structure
> > variables.  If a back-end does implement arrays using scratch or
> > indirects on registers, having more smaller arrays is likely to have
> > better memory efficiency.
> > ---
> >  src/compiler/Makefile.sources     |   1 +
> >  src/compiler/nir/meson.build      |   1 +
> >  src/compiler/nir/nir.h            |   1 +
> >  src/compiler/nir/nir_split_vars.c | 271 ++++++++++++++++++++++++++++++
> >  4 files changed, 274 insertions(+)
> >  create mode 100644 src/compiler/nir/nir_split_vars.c
>
> With the fix below, this patch is
>
> Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira at intel.com>
>
>
> > +static void
> > +init_field_for_type(struct field *field, struct field *parent,
> > +                    const struct glsl_type *type,
> > +                    const char *name,
> > +                    struct split_var_state *state)
> > +{
> > +   *field = (struct field) {
> > +      .parent = parent,
> > +      .type = type,
> > +   };
> > +
> > +   const struct glsl_type *struct_type = glsl_without_array(type);
> > +   if (glsl_type_is_struct(struct_type)) {
> > +      field->num_fields = glsl_get_length(struct_type),
> > +      field->fields = ralloc_array(state->mem_ctx, struct field,
> > +                                   field->num_fields);
> > +      for (unsigned i = 0; i < field->num_fields; i++) {
> > +         char *field_name = NULL;
> > +         if (name) {
> > +            ralloc_asprintf(state->mem_ctx, "%s_%s", name,
> > +                            glsl_get_struct_elem_name(struct_type, i));
>
> Store the result of asprintf:
>
>     field_name = ralloc_asprintf(...);
>

Fixed.


> And from previous review:
>
>     Maybe if no name for the parent is available, use something in
>     this place ("unnamed", or whatever).  That way the rest of the
>     hierarchy doesn't lose the meaning completely.
>

I've made it name the variable "{unnamed $type}" where $type is the name of
the structure type.

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180822/e7a14f1f/attachment.html>