<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Aug 20, 2018 at 6:41 PM Caio Marcelo de Oliveira Filho <<a href="mailto:caio.oliveira@intel.com">caio.oliveira@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Sat, Jul 28, 2018 at 10:44:36PM -0700, Jason Ekstrand wrote:<br>
> This pass doesn't really do much now because nir_lower_vars_to_ssa can<br>
> already see through structures and considers them to be "split". This<br>
> pass exists to help other passes more easily see through structure<br>
> variables. If a back-end does implement arrays using scratch or<br>
> indirects on registers, having more smaller arrays is likely to have<br>
> better memory efficiency.<br>
> ---<br>
> src/compiler/Makefile.sources | 1 +<br>
> src/compiler/nir/meson.build | 1 +<br>
> src/compiler/nir/nir.h | 1 +<br>
> src/compiler/nir/nir_split_vars.c | 271 ++++++++++++++++++++++++++++++<br>
> 4 files changed, 274 insertions(+)<br>
> create mode 100644 src/compiler/nir/nir_split_vars.c<br>
<br>
With the fix below, this patch is<br>
<br>
Reviewed-by: Caio Marcelo de Oliveira Filho <<a href="mailto:caio.oliveira@intel.com" target="_blank">caio.oliveira@intel.com</a>><br>
<br>
<br>
> +static void<br>
> +init_field_for_type(struct field *field, struct field *parent,<br>
> + const struct glsl_type *type,<br>
> + const char *name,<br>
> + struct split_var_state *state)<br>
> +{<br>
> + *field = (struct field) {<br>
> + .parent = parent,<br>
> + .type = type,<br>
> + };<br>
> +<br>
> + const struct glsl_type *struct_type = glsl_without_array(type);<br>
> + if (glsl_type_is_struct(struct_type)) {<br>
> + field->num_fields = glsl_get_length(struct_type),<br>
> + field->fields = ralloc_array(state->mem_ctx, struct field,<br>
> + field->num_fields);<br>
> + for (unsigned i = 0; i < field->num_fields; i++) {<br>
> + char *field_name = NULL;<br>
> + if (name) {<br>
> + ralloc_asprintf(state->mem_ctx, "%s_%s", name,<br>
> + glsl_get_struct_elem_name(struct_type, i));<br>
<br>
Store the result of asprintf:<br>
<br>
field_name = ralloc_asprintf(...);<br></blockquote><div><br></div><div>Fixed.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
And from previous review:<br>
<br>
Maybe if no name for the parent is available, use something in<br>
this place ("unnamed", or whatever). That way the rest of the<br>
hierarchy doesn't lose the meaning completely.<br></blockquote><div><br></div><div>I've made it name the variable "{unnamed $type}" where $type is the name of the structure type.</div><div><br></div><div>--Jason<br></div></div></div>