[Mesa-dev] [PATCH 1/7] glsl: only set buffer block binding once during initialisation

[Kenneth Graunke]

On Saturday, April 2, 2016 3:03:52 PM PDT Timothy Arceri wrote:
> Since 8683d54d2be825 there is now a single instance of the buffer
> block information that needs to be updated rather than one instance
> for each stage.
> ---
>  src/compiler/glsl/link_uniform_initializers.cpp | 32 ++++
+--------------------
>  1 file changed, 6 insertions(+), 26 deletions(-)
> 
> diff --git a/src/compiler/glsl/link_uniform_initializers.cpp b/src/compiler/
glsl/link_uniform_initializers.cpp
> index 870bc5b..f40ec19 100644
> --- a/src/compiler/glsl/link_uniform_initializers.cpp
> +++ b/src/compiler/glsl/link_uniform_initializers.cpp
> @@ -44,18 +44,6 @@ get_storage(gl_uniform_storage *storage, unsigned 
num_storage,
>     return NULL;
>  }
>  
> -static unsigned
> -get_uniform_block_index(const gl_shader_program *shProg,
> -                        const char *uniformBlockName)
> -{
> -   for (unsigned i = 0; i < shProg->NumBufferInterfaceBlocks; i++) {
> -      if (!strcmp(shProg->BufferInterfaceBlocks[i].Name, uniformBlockName))
> -	 return i;
> -   }
> -
> -   return GL_INVALID_INDEX;
> -}
> -
>  void
>  copy_constant_to_storage(union gl_constant_value *storage,
>  			 const ir_constant *val,
> @@ -168,22 +156,14 @@ set_opaque_binding(void *mem_ctx, gl_shader_program 
*prog,
>  void
>  set_block_binding(gl_shader_program *prog, const char *block_name, int 
binding)
>  {
> -   const unsigned block_index = get_uniform_block_index(prog, block_name);
> -
> -   if (block_index == GL_INVALID_INDEX) {
> -      assert(block_index != GL_INVALID_INDEX);
> -      return;
> +   for (unsigned i = 0; i < prog->NumBufferInterfaceBlocks; i++) {
> +      if (!strcmp(prog->BufferInterfaceBlocks[i].Name, block_name))
> +         prog->BufferInterfaceBlocks[i].Binding = binding;
> +	 return;

Err...it looks like you're missing braces here.  Won't this just
unconditionally return after looking at the first list element?

>     }
>  
> -      /* This is a field of a UBO.  val is the binding index. */
> -      for (int i = 0; i < MESA_SHADER_STAGES; i++) {
> -         int stage_index = prog->InterfaceBlockStageIndex[i][block_index];
> -
> -         if (stage_index != -1) {
> -            struct gl_shader *sh = prog->_LinkedShaders[i];
> -            sh->BufferInterfaceBlocks[stage_index]->Binding = binding;
> -         }
> -      }
> +   assert(!"Failed to initilise block binding");
> +   return;

Maybe make this unreachable("Failed to initialize block binding");
and drop the unnecessary return...

With those two changes, and assuming testing is still happy,
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160401/8cb784a4/attachment.sig>