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

[Caio Marcelo de Oliveira Filho]

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(...);

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.



Thanks,
Caio