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

2013-03-25 Thread Pohjolainen, Topi 
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

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

2013-03-23 Thread Kenneth Graunke 

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.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org

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

2013-03-19 Thread Pohjolainen, Topi 
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.

 +   }
 +
 +   new_var-interface_type = iface_t;
 hash_table_insert(interface_namespace, new_var,
   iface_field_name);
 insert_pos-insert_after(new_var);
 @@ -187,9 +212,19 @@ 
 flatten_named_interface_blocks_declarations::handle_rvalue(ir_rvalue **rvalue)
   (ir_variable *) hash_table_find(interface_namespace,
   iface_field_name);
assert(found_var);
 +
ir_dereference_variable *deref_var =
   new(mem_ctx) ir_dereference_variable(found_var);
 -  *rvalue = deref_var;
 +
 +  ir_dereference_array *deref_array =
 + ir-record-as_dereference_array();
 +  if (deref_array != NULL) {
 + *rvalue =
 +new(mem_ctx) ir_dereference_array(deref_var,
 +  deref_array-array_index);
 +  } else {
 + *rvalue = deref_var;
 +  }
 }
  }
  
 -- 
 1.7.10.4
 
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2013-03-19 Thread Jordan Justen 
On Tue, Mar 19, 2013 at 3:57 AM, Pohjolainen, Topi
topi.pohjolai...@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.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.

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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2013-03-18 Thread Jordan Justen 
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;
+   }
+
+   new_var-interface_type = iface_t;
hash_table_insert(interface_namespace, new_var,
  iface_field_name);
insert_pos-insert_after(new_var);
@@ -187,9 +212,19 @@ 
flatten_named_interface_blocks_declarations::handle_rvalue(ir_rvalue **rvalue)
  (ir_variable *) hash_table_find(interface_namespace,
  iface_field_name);
   assert(found_var);
+
   ir_dereference_variable *deref_var =
  new(mem_ctx) ir_dereference_variable(found_var);
-  *rvalue = deref_var;
+
+  ir_dereference_array *deref_array =
+ ir-record-as_dereference_array();
+  if (deref_array != NULL) {
+ *rvalue =
+new(mem_ctx) ir_dereference_array(deref_var,
+  deref_array-array_index);
+  } else {
+ *rvalue = deref_var;
+  }
}
 }
 
-- 
1.7.10.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev