[Mesa-dev] [RFC 2/2] mesa: Add {Num}UniformBlocks and {Num}ShaderStorageBlocks to gl_shader_program

[Ilia Mirkin]

On Thu, Oct 1, 2015 at 7:09 AM, Iago Toral Quiroga <itoral at igalia.com> wrote:
> These arrays provide backends with separate index spaces for UBOS and SSBOs.
> ---
>  src/glsl/linker.cpp                 | 35 +++++++++++++++++++++++++++++++++++
>  src/glsl/standalone_scaffolding.cpp |  9 +++++++++
>  src/mesa/main/mtypes.h              |  6 ++++++
>  3 files changed, 50 insertions(+)
>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index e6eba94..3da773d 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -4107,6 +4107,41 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>        }
>     }
>
> +   /* Split prog->BufferInterfaceBlocks into prog->UniformBlocks and
> +    * prog->ShaderStorageBlocks, so that drivers that need separate index
> +    * spaces for each set can have that.
> +    */
> +   unsigned num_ubo_blocks;
> +   unsigned num_ssbo_blocks;
> +   num_ubo_blocks = 0;
> +   num_ssbo_blocks = 0;
> +   for (unsigned i = 0; i < prog->NumBufferInterfaceBlocks; i++) {
> +      if (prog->BufferInterfaceBlocks[i].IsShaderStorage)
> +         num_ssbo_blocks++;
> +      else
> +         num_ubo_blocks++;
> +   }
> +
> +   prog->UniformBlocks =
> +      ralloc_array(mem_ctx, gl_uniform_block *, num_ubo_blocks);
> +   prog->NumUniformBlocks = 0;
> +
> +   prog->ShaderStorageBlocks =
> +      ralloc_array(mem_ctx, gl_uniform_block *, num_ssbo_blocks);
> +   prog->NumShaderStorageBlocks = 0;
> +
> +   for (unsigned i = 0; i < prog->NumBufferInterfaceBlocks; i++) {
> +      if (prog->BufferInterfaceBlocks[i].IsShaderStorage)
> +         prog->ShaderStorageBlocks[prog->NumShaderStorageBlocks++] =
> +            &prog->BufferInterfaceBlocks[i];
> +      else
> +         prog->UniformBlocks[prog->NumUniformBlocks++] =
> +            &prog->BufferInterfaceBlocks[i];
> +   }

Shouldn't this go through and also adjust the indices of the linked
programs? Or... something along those lines? With this, I still need a
remapping table to go from the index passed to a load_ssbo/load_ubo
instruction to a ssbo/ubo index.

Perhaps a few well-placed helper functions can alleviate this? Also
this should erase the need for some of the O(n) iterations that have
sprung up as a result of this combined list.

IMHO ideally the BufferInterfaceBlocks list would get freed at the end
of this function. But I understand that this will require work, and
the onus is probably on me (or anyone wanting to add ssbo support to
other drivers) to do it, or work around it.

  -ilia

> +
> +   assert(prog->NumUniformBlocks + prog->NumShaderStorageBlocks ==
> +          prog->NumBufferInterfaceBlocks);
> +
>     /* FINISHME: Assign fragment shader output locations. */
>
>  done:
> diff --git a/src/glsl/standalone_scaffolding.cpp b/src/glsl/standalone_scaffolding.cpp
> index 0c53589..658245f 100644
> --- a/src/glsl/standalone_scaffolding.cpp
> +++ b/src/glsl/standalone_scaffolding.cpp
> @@ -102,6 +102,15 @@ _mesa_clear_shader_program_data(struct gl_shader_program *shProg)
>     ralloc_free(shProg->BufferInterfaceBlocks);
>     shProg->BufferInterfaceBlocks = NULL;
>     shProg->NumBufferInterfaceBlocks = 0;
> +
> +   ralloc_free(shProg->UniformBlocks);
> +   shProg->UniformBlocks = NULL;
> +   shProg->NumUniformBlocks = 0;
> +
> +   ralloc_free(shProg->ShaderStorageBlocks);
> +   shProg->ShaderStorageBlocks = NULL;
> +   shProg->NumShaderStorageBlocks = 0;
> +
>     for (i = 0; i < MESA_SHADER_STAGES; i++) {
>        ralloc_free(shProg->UniformBlockStageIndex[i]);
>        shProg->UniformBlockStageIndex[i] = NULL;
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 347da14..2362f54 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2692,6 +2692,12 @@ struct gl_shader_program
>     unsigned NumBufferInterfaceBlocks;
>     struct gl_uniform_block *BufferInterfaceBlocks;
>
> +   unsigned NumUniformBlocks;
> +   struct gl_uniform_block **UniformBlocks;
> +
> +   unsigned NumShaderStorageBlocks;
> +   struct gl_uniform_block **ShaderStorageBlocks;
> +
>     /**
>      * Indices into the _LinkedShaders's UniformBlocks[] array for each stage
>      * they're used in, or -1.
> --
> 1.9.1
>