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

[Samuel Iglesias Gonsálvez]

On 09/06/15 20:19, Jordan Justen wrote:
> 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>
> 

OK, I will fix it.

Thanks!

Sam

>>        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
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>