[Mesa-dev] [PATCH v2 13/15] glsl linker: support arrays of interface block instances

[Jordan Justen]

On Tue, Mar 19, 2013 at 3:57 AM, Pohjolainen, Topi
<topi.pohjolainen at intel.com> wrote:
> On Mon, Mar 18, 2013 at 04:35:10PM -0700, Jordan Justen wrote:
>> With this change we now support interface block arrays.
>> For example, cases like this:
>>
>> out block_name {
>>     float f;
>> } block_instance[2];
>>
>> This allows Mesa to pass the piglit glsl-1.50 test:
>> * execution/interface-blocks-complex-vs-fs.shader_test
>>
>> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
>> ---
>>  src/glsl/lower_named_interface_blocks.cpp |   53 ++++++++++++++++++++++++-----
>>  1 file changed, 44 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/glsl/lower_named_interface_blocks.cpp b/src/glsl/lower_named_interface_blocks.cpp
>> index 2e0c322..405e7a9 100644
>> --- a/src/glsl/lower_named_interface_blocks.cpp
>> +++ b/src/glsl/lower_named_interface_blocks.cpp
>> @@ -107,22 +107,47 @@ flatten_named_interface_blocks_declarations::run(exec_list *instructions)
>>           if (var->mode == ir_var_uniform)
>>              continue;
>>
>> -         const glsl_type *const t = var->type;
>> +         const glsl_type * iface_t = var->type;
>> +         const glsl_type * array_t = NULL;
>>           exec_node *insert_pos = var;
>> -         char *iface_field_name;
>> -         for (unsigned i = 0; i < t->length; i++) {
>> -            iface_field_name = ralloc_asprintf(mem_ctx, "%s.%s", t->name,
>> -                                               t->fields.structure[i].name);
>> +
>> +         if (iface_t->is_array()) {
>> +            array_t = iface_t;
>> +            iface_t = array_t->fields.array;
>> +         }
>> +
>> +         assert (iface_t->is_interface());
>> +
>> +         for (unsigned i = 0; i < iface_t->length; i++) {
>> +            const char * field_name = iface_t->fields.structure[i].name;
>> +            char *iface_field_name =
>> +               ralloc_asprintf(mem_ctx, "%s.%s",
>> +                               iface_t->name, field_name);
>>
>>              ir_variable *found_var =
>>                 (ir_variable *) hash_table_find(interface_namespace,
>>                                                 iface_field_name);
>>              if (!found_var) {
>>                 ir_variable *new_var =
>> -                  new(mem_ctx) ir_variable(t->fields.structure[i].type,
>> -                                           ralloc_strdup(mem_ctx, t->fields.structure[i].name),
>> +                  new(mem_ctx) ir_variable(iface_t->fields.structure[i].type,
>> +                                           ralloc_strdup(mem_ctx, iface_t->fields.structure[i].name),
>>                                             (ir_variable_mode) var->mode);
>> -               new_var->interface_type = t;
>> +               if (array_t != NULL) {
>> +                  const glsl_type *new_array_type =
>> +                     glsl_type::get_array_instance(
>> +                        iface_t->fields.structure[i].type,
>> +                        array_t->length);
>> +                  char *array_var_name =
>> +                     ralloc_asprintf(mem_ctx, "%s[%d]",
>> +                                     new_var->name, array_t->length);
>> +                  ir_variable *new_array_var =
>> +                     new(mem_ctx) ir_variable(new_array_type,
>> +                                              array_var_name,
>> +                                              (ir_variable_mode) var->mode);
>> +                  new_var = new_array_var;
>
> Don't you leak the previously allocated instance of 'ir_variable' (assigned to
> 'new_var')? Or is it just left until 'mem_ctx' gets released?
> I'm not that familiar with the glsl core that I may well be missing something.

I think ralloc would have mostly saved me on that. :)

But you are right. I don't need to create the ir_variable in the array
case. I'll fix it.

Thanks,

-Jordan