[Mesa-dev] [PATCH 12/32] glsl: Add ir_variable::interface_type field

[Paul Berry]

On 22 January 2013 00:52, Ian Romanick <idr at freedesktop.org> wrote:

> From: Ian Romanick <ian.d.romanick at intel.com>
>
> For variables that are in an interface block or are an instance of an
> interface block, this is the GLSL_TYPE_INTERFACE type for that block.
>
> Convert the ir_variable::is_in_uniform_block method added in the
> previous commit to use this field instead of ir_variable::uniform_block.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  src/glsl/ast_to_hir.cpp | 2 ++
>  src/glsl/ir.h           | 9 ++++++++-
>  src/glsl/ir_clone.cpp   | 2 ++
>  3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index a740a3c..47a1ae0 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -4254,6 +4254,7 @@ ast_uniform_block::hir(exec_list *instructions,
>                                                  this->instance_name,
>                                                  ir_var_uniform);
>
> +      var->interface_type = block_type;
>        state->symbols->add_variable(var);
>        instructions->push_tail(var);
>     } else {
> @@ -4263,6 +4264,7 @@ ast_uniform_block::hir(exec_list *instructions,
>                                     ralloc_strdup(state, fields[i].name),
>                                     ir_var_uniform);
>           var->uniform_block = ubo - state->uniform_blocks;
> +         var->interface_type = block_type;
>
>           state->symbols->add_variable(var);
>           instructions->push_tail(var);
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index a7eb9c1..49c5f8d 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -352,7 +352,7 @@ public:
>      */
>     inline bool is_in_uniform_block() const
>     {
> -      return this->mode == ir_var_uniform && this->uniform_block != -1;
> +      return this->mode == ir_var_uniform && this->interface_type != NULL;
>     }
>
>     /**
> @@ -538,6 +538,13 @@ public:
>      * objects.
>      */
>     ir_constant *constant_initializer;
> +
> +   /**
> +    * interface
> +    *
> +    * \sa ir_variable::location
> +    */
> +   const glsl_type *interface_type;
>

This comment doesn't really help me understand what this field means.

How about putting in this text (from the commit message):

For variables that are in an interface block or are an instance of an
interface block, this is the GLSL_TYPE_INTERFACE type for that block.


>  };
>
>
> diff --git a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp
> index 3e22f2d..c221a96 100644
> --- a/src/glsl/ir_clone.cpp
> +++ b/src/glsl/ir_clone.cpp
> @@ -77,6 +77,8 @@ ir_variable::clone(void *mem_ctx, struct hash_table *ht)
> const
>        var->constant_initializer =
>          this->constant_initializer->clone(mem_ctx, ht);
>
> +   var->interface_type = this->interface_type;
> +
>     if (ht) {
>        hash_table_insert(ht, var, (void *)const_cast<ir_variable *>(this));
>     }
> --
> 1.7.11.7
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>

I'm not certain whether it's strictly necessary, but I would feel much more
confident in this patch if we also initialized interface_type to NULL in
the ir_variable constructor.

With those two changes, this patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130123/269eeaf2/attachment.html>