[Mesa-dev] [PATCH v2 11/82] glsl: enable binding layout qualifier usage for shader storage buffer objects

[Jordan Justen]

On 2015-06-03 00:01:01, Iago Toral Quiroga wrote:
> From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> 
> See GLSL 4.30 spec, section 4.4.5 "Uniform and Shader Storage Block
> Layout Qualifiers".
> 
> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> ---
>  src/glsl/ast_to_hir.cpp | 29 ++++++++++++++++++++++++-----
>  src/glsl/glsl_parser.yy |  3 ++-
>  2 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 63ab56a..73d3c78 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2044,9 +2044,10 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
>                             ir_variable *var,
>                             const ast_type_qualifier *qual)
>  {
> -   if (var->data.mode != ir_var_uniform) {
> +   if (var->data.mode != ir_var_uniform && var->data.mode != ir_var_shader_storage) {
>        _mesa_glsl_error(loc, state,
> -                       "the \"binding\" qualifier only applies to uniforms");
> +                       "the \"binding\" qualifier only applies to uniforms and"
> +                       "shader storage buffer objects.");

Missing space between 'and' and 'shader'. I think we usually don't
include a period (.) at the end of these messages.

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

>        return false;
>     }
>  
> @@ -2070,13 +2071,31 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
>         *
>         * The implementation-dependent maximum is GL_MAX_UNIFORM_BUFFER_BINDINGS.
>         */
> -      if (max_index >= ctx->Const.MaxUniformBufferBindings) {
> +      if (var->data.mode == ir_var_uniform &&
> +         max_index >= ctx->Const.MaxUniformBufferBindings) {
>           _mesa_glsl_error(loc, state, "layout(binding = %d) for %d UBOs exceeds "
>                            "the maximum number of UBO binding points (%d)",
>                            qual->binding, elements,
>                            ctx->Const.MaxUniformBufferBindings);
>           return false;
>        }
> +      /* SSBOs. From page 67 of the GLSL 4.30 specification:
> +       * "If the binding point for any uniform or shader storage block instance
> +       *  is less than zero, or greater than or equal to the
> +       *  implementation-dependent maximum number of uniform buffer bindings, a
> +       *  compile-time error will occur. When the binding identifier is used
> +       *  with a uniform or shader storage block instanced as an array of size
> +       *  N, all elements of the array from binding through binding + N – 1 must
> +       *  be within this range."
> +       */
> +      if (var->data.mode == ir_var_shader_storage &&
> +         max_index >= ctx->Const.MaxShaderStorageBufferBindings) {
> +         _mesa_glsl_error(loc, state, "layout(binding = %d) for %d SSBOs exceeds "
> +                          "the maximum number of SSBO binding points (%d)",
> +                          qual->binding, elements,
> +                          ctx->Const.MaxShaderStorageBufferBindings);
> +         return false;
> +      }
>     } else if (var->type->is_sampler() ||
>                (var->type->is_array() && var->type->fields.array->is_sampler())) {
>        /* Samplers.  From page 63 of the GLSL 4.20 specification:
> @@ -5852,8 +5871,8 @@ ast_interface_block::hir(exec_list *instructions,
>           if (state->symbols->get_variable(var->name) != NULL)
>              _mesa_glsl_error(&loc, state, "`%s' redeclared", var->name);
>  
> -         /* Propagate the "binding" keyword into this UBO's fields;
> -          * the UBO declaration itself doesn't get an ir_variable unless it
> +         /* Propagate the "binding" keyword into this UBO/SSBO's fields.
> +          * The UBO declaration itself doesn't get an ir_variable unless it
>            * has an instance name.  This is ugly.
>            */
>           var->data.explicit_binding = this->layout.flags.q.explicit_binding;
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index 8564cb9..37c4401 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -1425,7 +1425,8 @@ layout_qualifier_id:
>        }
>  
>        if ((state->ARB_shading_language_420pack_enable ||
> -           state->has_atomic_counters()) &&
> +           state->has_atomic_counters() ||
> +           state->ARB_shader_storage_buffer_object_enable) &&
>            match_layout_qualifier("binding", $1, state) == 0) {
>           $$.flags.q.explicit_binding = 1;
>           $$.binding = $3;
> -- 
> 1.9.1
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev