[Mesa-dev] [PATCH 06/16] glsl: Do not eliminate 'shared' or 'std140' blocks or block members

[Matt Turner]

On Mon, Jul 21, 2014 at 2:04 PM, Ian Romanick <idr at freedesktop.org> wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> Commit 32f32292 (glsl: Allow elimination of uniform block members)
> enabled elimination of unused uniform block members to fix a gles3
> conformance test failure.  This went too far the other way.
>
> Section 2.11.6 (Uniform Variables) of the OpenGL ES 3.0.3 spec says:
>
>     "All members of a named uniform block declared with a shared or
>     std140 layout qualifier are considered active, even if they are not
>     referenced in any shader in the program. The uniform block itself is
>     also considered active, even if no member of the block is
>     referenced."
>
> Fixes gles3conform failures in:
>
> ES3-CTS.shaders.uniform_block.single_nested_struct.per_block_buffer_shared
> ES3-CTS.shaders.uniform_block.single_nested_struct.per_block_buffer_std140
> ES3-CTS.shaders.uniform_block.single_nested_struct_array.per_block_buffer_shared
> ES3-CTS.shaders.uniform_block.single_nested_struct_array.per_block_buffer_std140
> ES3-CTS.shaders.uniform_block.random.scalar_types.2
> ES3-CTS.shaders.uniform_block.random.scalar_types.9
> ES3-CTS.shaders.uniform_block.random.vector_types.1
> ES3-CTS.shaders.uniform_block.random.vector_types.3
> ES3-CTS.shaders.uniform_block.random.vector_types.7
> ES3-CTS.shaders.uniform_block.random.vector_types.9
> ES3-CTS.shaders.uniform_block.random.basic_types.5
> ES3-CTS.shaders.uniform_block.random.basic_types.6
> ES3-CTS.shaders.uniform_block.random.basic_arrays.0
> ES3-CTS.shaders.uniform_block.random.basic_arrays.2
> ES3-CTS.shaders.uniform_block.random.basic_arrays.5
> ES3-CTS.shaders.uniform_block.random.basic_arrays.8
> ES3-CTS.shaders.uniform_block.random.basic_instance_arrays.0
> ES3-CTS.shaders.uniform_block.random.basic_instance_arrays.4
> ES3-CTS.shaders.uniform_block.random.basic_instance_arrays.5
> ES3-CTS.shaders.uniform_block.random.basic_instance_arrays.6
> ES3-CTS.shaders.uniform_block.random.basic_instance_arrays.9
> ES3-CTS.shaders.uniform_block.random.nested_structs.0
> ES3-CTS.shaders.uniform_block.random.nested_structs.1
> ES3-CTS.shaders.uniform_block.random.nested_structs_arrays.4
> ES3-CTS.shaders.uniform_block.random.nested_structs_instance_arrays.8
> ES3-CTS.shaders.uniform_block.random.nested_structs_arrays_instance_arrays.7
> ES3-CTS.shaders.uniform_block.random.all_per_block_buffers.3
> ES3-CTS.shaders.uniform_block.random.all_per_block_buffers.6
> ES3-CTS.shaders.uniform_block.random.all_shared_buffer.18
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  src/glsl/link_uniform_block_active_visitor.cpp | 39 ++++++++++++++++++++++++++
>  src/glsl/link_uniform_block_active_visitor.h   |  1 +
>  src/glsl/opt_dead_code.cpp                     | 29 ++++++++++++++++---
>  3 files changed, 65 insertions(+), 4 deletions(-)
>
> diff --git a/src/glsl/link_uniform_block_active_visitor.cpp b/src/glsl/link_uniform_block_active_visitor.cpp
> index d19ce20..c754ef6 100644
> --- a/src/glsl/link_uniform_block_active_visitor.cpp
> +++ b/src/glsl/link_uniform_block_active_visitor.cpp
> @@ -73,6 +73,45 @@ process_block(void *mem_ctx, struct hash_table *ht, ir_variable *var)
>  }
>
>  ir_visitor_status
> +link_uniform_block_active_visitor::visit(ir_variable *var)
> +{
> +   if (!var->is_in_uniform_block())
> +      return visit_continue;
> +
> +   const glsl_type *const block_type = var->is_interface_instance()
> +      ? var->type : var->get_interface_type();
> +
> +   /* Section 2.11.6 (Uniform Variables) of the OpenGL ES 3.0.3 spec says:
> +    *
> +    *     "All members of a named uniform block declared with a shared or
> +    *     std140 layout qualifier are considered active, even if they are not
> +    *     referenced in any shader in the program. The uniform block itself is
> +    *     also considered active, even if no member of the block is
> +    *     referenced."
> +    */
> +   if (block_type->interface_packing == GLSL_INTERFACE_PACKING_PACKED)
> +      return visit_continue;
> +
> +   /* Process the block.  Bail if there was an error.
> +    */
> +   link_uniform_block_active *const b =
> +      process_block(this->mem_ctx, this->ht, var);
> +   if (b == NULL) {
> +      linker_error(this->prog,
> +                  "uniform block `%s' has mismatching definitions",
> +                  var->get_interface_type()->name);

Tabs.

> +      this->success = false;
> +      return visit_stop;
> +   }
> +
> +   assert(b->num_array_elements == 0);
> +   assert(b->array_elements == NULL);
> +   assert(b->type != NULL);
> +
> +   return visit_continue;
> +}
> +
> +ir_visitor_status
>  link_uniform_block_active_visitor::visit_enter(ir_dereference_array *ir)
>  {
>     ir_dereference_variable *const d = ir->array->as_dereference_variable();
> diff --git a/src/glsl/link_uniform_block_active_visitor.h b/src/glsl/link_uniform_block_active_visitor.h
> index 524cd6b..ae3461c 100644
> --- a/src/glsl/link_uniform_block_active_visitor.h
> +++ b/src/glsl/link_uniform_block_active_visitor.h
> @@ -51,6 +51,7 @@ public:
>
>     virtual ir_visitor_status visit_enter(ir_dereference_array *);
>     virtual ir_visitor_status visit(ir_dereference_variable *);
> +   virtual ir_visitor_status visit(class ir_variable *);

No need for class keyword?

>
>     bool success;
>
> diff --git a/src/glsl/opt_dead_code.cpp b/src/glsl/opt_dead_code.cpp
> index da90bfb..3cec7d4 100644
> --- a/src/glsl/opt_dead_code.cpp
> +++ b/src/glsl/opt_dead_code.cpp
> @@ -99,10 +99,31 @@ do_dead_code(exec_list *instructions, bool uniform_locations_assigned)
>           * stage.  Also, once uniform locations have been assigned, the
>           * declaration cannot be deleted.
>           */
> -        if (entry->var->data.mode == ir_var_uniform &&
> -            (uniform_locations_assigned ||
> -             entry->var->constant_value))
> -           continue;
> +        if (entry->var->data.mode == ir_var_uniform) {
> +            if (uniform_locations_assigned || entry->var->constant_value)
> +               continue;
> +
> +            /* Section 2.11.6 (Uniform Variables) of the OpenGL ES 3.0.3 spec
> +             * says:
> +             *
> +             *     "All members of a named uniform block declared with a
> +             *     shared or std140 layout qualifier are considered active,
> +             *     even if they are not referenced in any shader in the
> +             *     program. The uniform block itself is also considered
> +             *     active, even if no member of the block is referenced."
> +             *
> +             * if the variable is in a uniform block with one of those

Since I'm already nitpicking... capitalize if?