[Mesa-dev] [PATCH] glsl: fix lowering of UBO references of named blocks

[Iago Toral]

On Mon, 2016-10-31 at 22:19 +0100, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
> 
> When a UBO reference has the form block_name.foo where block_name
> refers
> to a block where the first member has a non-zero offset, the base
> offset
> was incorrectly added to the reference.

The patch looks good to me and is:

Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

I am curious: in what scenario doesn't the first member of a block have
a zero offset?

Iago

> Fixes an assertion triggered in debug builds by
> GL45-CTS.enhanced_layouts.uniform_block_layout_qualifier_conflict.
> That test
> doesn't properly check for correct execution in this case, so I am
> also
> going to send out a piglit test.
> 
> Cc: 13.0 <mesa-stable at lists.freedesktop.org>
> ---
>  src/compiler/glsl/lower_ubo_reference.cpp | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/compiler/glsl/lower_ubo_reference.cpp
> b/src/compiler/glsl/lower_ubo_reference.cpp
> index 37134a9..eafa1dd 100644
> --- a/src/compiler/glsl/lower_ubo_reference.cpp
> +++ b/src/compiler/glsl/lower_ubo_reference.cpp
> @@ -100,21 +100,20 @@ public:
>  
>     unsigned calculate_unsized_array_stride(ir_dereference *deref,
>                                             enum
> glsl_interface_packing packing);
>  
>     ir_call *lower_ssbo_atomic_intrinsic(ir_call *ir);
>     ir_call *check_for_ssbo_atomic_intrinsic(ir_call *ir);
>     ir_visitor_status visit_enter(ir_call *ir);
>  
>     struct gl_linked_shader *shader;
>     bool clamp_block_indices;
> -   struct gl_uniform_buffer_variable *ubo_var;
>     const struct glsl_struct_field *struct_field;
>     ir_variable *variable;
>     ir_rvalue *uniform_block;
>     bool progress;
>  };
>  
>  /**
>   * Determine the name of the interface block field
>   *
>   * This is the name of the specific member as it would appear in the
> @@ -301,31 +300,32 @@
> lower_ubo_reference_visitor::setup_for_load_or_store(void *mem_ctx,
>        if (strcmp(field_name, blocks[i]->Name) == 0) {
>  
>           ir_constant *index = new(mem_ctx) ir_constant(i);
>  
>           if (nonconst_block_index) {
>              this->uniform_block = add(nonconst_block_index, index);
>           } else {
>              this->uniform_block = index;
>           }
>  
> -         this->ubo_var = var->is_interface_instance()
> -            ? &blocks[i]->Uniforms[0] : &blocks[i]->Uniforms[var-
> >data.location];
> +         if (var->is_interface_instance()) {
> +            *const_offset = 0;
> +         } else {
> +            *const_offset = blocks[i]->Uniforms[var-
> >data.location].Offset;
> +         }
>  
>           break;
>        }
>     }
>  
>     assert(this->uniform_block);
>  
> -   *const_offset = ubo_var->Offset;
> -
>     this->struct_field = NULL;
>     setup_buffer_access(mem_ctx, deref, offset, const_offset,
> row_major,
>                         matrix_columns, &this->struct_field,
> packing);
>  }
>  
>  void
>  lower_ubo_reference_visitor::handle_rvalue(ir_rvalue **rvalue)
>  {
>     if (!*rvalue)
>        return;