[Mesa-dev] [PATCH v4 (part2) 24/59] glsl: a shader storage buffer must be smaller than the maximum size allowed

[Jordan Justen]

On 2015-08-05 01:30:21, Iago Toral Quiroga wrote:
> From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> 
> Otherwise, generate a link time error as per the
> ARB_shader_storage_buffer_object spec.
> 
> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> ---
>  src/glsl/glsl_types.cpp          |  9 +++++++--
>  src/glsl/link_uniform_blocks.cpp | 17 +++++++++++++++++
>  src/glsl/linker.cpp              |  2 +-
>  src/glsl/linker.h                |  1 +
>  4 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 5758029..1b8e2e4 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -1312,7 +1312,7 @@ glsl_type::std140_size(bool row_major) const
>      *     rounded up to the next multiple of the base alignment of the
>      *     structure.
>      */
> -   if (this->is_record()) {
> +   if (this->is_record() || this->is_interface()) {
>        unsigned size = 0;
>        unsigned max_align = 0;
>  
> @@ -1328,7 +1328,12 @@ glsl_type::std140_size(bool row_major) const
>  
>          const struct glsl_type *field_type = this->fields.structure[i].type;
>          unsigned align = field_type->std140_base_alignment(field_row_major);
> -        size = glsl_align(size, align);
> +
> +         /* Ignore unsized arrays when calculating size */
> +         if (field_type->is_unsized_array())
> +            continue;
> +
> +         size = glsl_align(size, align);
>          size += field_type->std140_size(field_row_major);
>  
>           max_align = MAX2(align, max_align);
> diff --git a/src/glsl/link_uniform_blocks.cpp b/src/glsl/link_uniform_blocks.cpp
> index c891d03..2cda78d 100644
> --- a/src/glsl/link_uniform_blocks.cpp
> +++ b/src/glsl/link_uniform_blocks.cpp
> @@ -178,6 +178,7 @@ struct block {
>  
>  unsigned
>  link_uniform_blocks(void *mem_ctx,
> +                    struct gl_context *ctx,
>                      struct gl_shader_program *prog,
>                      struct gl_shader **shader_list,
>                      unsigned num_shaders,
> @@ -299,6 +300,14 @@ link_uniform_blocks(void *mem_ctx,
>  
>              blocks[i].UniformBufferSize = parcel.buffer_size;
>  
> +            /* Check SSBO size is lower than maximum supported size for SSBO */
> +            if (b->is_shader_storage &&
> +                parcel.buffer_size > ctx->Const.MaxShaderStorageBlockSize) {
> +               linker_error(prog, "shader storage block `%s' has size %d > %d",
> +                           block_type->name,
> +                           parcel.buffer_size,
> +                           ctx->Const.MaxShaderStorageBlockSize);

This error message (and below) seems confusing. What about "shader
storage block `%s' has size %d, which is larger than than the maximum
allowed (%d)"?

Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

> +            }
>              blocks[i].NumUniforms =
>                 (unsigned)(ptrdiff_t)(&variables[parcel.index] - blocks[i].Uniforms);
>  
> @@ -319,6 +328,14 @@ link_uniform_blocks(void *mem_ctx,
>  
>           blocks[i].UniformBufferSize = parcel.buffer_size;
>  
> +         /* Check SSBO size is lower than maximum supported size for SSBO */
> +         if (b->is_shader_storage &&
> +             parcel.buffer_size > ctx->Const.MaxShaderStorageBlockSize) {
> +            linker_error(prog, "shader storage block `%s' has size %d > %d",
> +                        block_type->name,
> +                        parcel.buffer_size,
> +                        ctx->Const.MaxShaderStorageBlockSize);
> +         }
>           blocks[i].NumUniforms =
>              (unsigned)(ptrdiff_t)(&variables[parcel.index] - blocks[i].Uniforms);
>  
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 9aff3ce..7ba685b 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -2023,7 +2023,7 @@ link_intrastage_shaders(void *mem_ctx,
>  
>     /* Link up uniform blocks defined within this stage. */
>     const unsigned num_uniform_blocks =
> -      link_uniform_blocks(mem_ctx, prog, shader_list, num_shaders,
> +      link_uniform_blocks(mem_ctx, ctx, prog, shader_list, num_shaders,
>                            &uniform_blocks);
>     if (!prog->LinkStatus)
>        return NULL;
> diff --git a/src/glsl/linker.h b/src/glsl/linker.h
> index ce3dc32..ada5a1f 100644
> --- a/src/glsl/linker.h
> +++ b/src/glsl/linker.h
> @@ -56,6 +56,7 @@ link_uniform_blocks_are_compatible(const gl_uniform_block *a,
>  
>  extern unsigned
>  link_uniform_blocks(void *mem_ctx,
> +                    struct gl_context *ctx,
>                      struct gl_shader_program *prog,
>                      struct gl_shader **shader_list,
>                      unsigned num_shaders,
> -- 
> 1.9.1
>