On Sat, Mar 23, 2013 at 01:48:44PM -0700, Kenneth Graunke wrote:
> On 03/19/2013 03:57 AM, Pohjolainen, Topi 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.jus...@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.
>
> This is actually fairly common in the GLSL code - we routinely just
> drop allocated memory on the floor. But, it all works out thanks to
> the magic of talloc/ralloc(*).
>
> With talloc, you allocate memory out af a 'context'. Freeing a
> context frees any memory associated with it, which means you can
> free a whole bunch of things without tracking them all down and
> calling free() on each individual pointer. It also means you don't
> need to explicitly keep pointers to each object, as the memory
> context does that for you.
>
> Any talloced piece of memory is also a new context, which allows you
> to create a tree-like structure (the 't' in talloc is for 'tree',
> and the 'r' in ralloc is for 'recursive').
>
> In the GLSL compiler, we allocate IR out of either the parser state
> or the gl_shader object (I forget which). During optimization,
> linking, and so on, we create new IR, and remove other IR, all
> without worrying about memory. Then, when we're done
> compiling/linking, we walk through the remaining IR tree, calling
> talloc_steal() on each remaining IR node to reparent the memory to a
> second memory context. Then we free the original memory context,
> which now contains only the junk we don't need anymore.
>
> (*) ralloc is my poor man's reimplementation of talloc, licensed
> under the MIT license rather than LGPLv3. It also has a slightly
> different API and performance characteristics.
Yes, I took a look at the implemention just after asking about it, and
understood that it is indeed the 'mem_ctx' bookkeeping that this "dropping on
the floor" is relying on as you explained. The implentation does, however,
allow one to use it without a context (mem_ctx == 0) and in that case there
would be nothing keeping track and hence the things "on the floor" getting
leaked. But I assume there is nothing using this logic without a proper memory
context.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
