On 02/09/2016 06:06 AM, Samuel Iglesias Gonsálvez wrote:
> 
> On Tue, 2016-01-12 at 20:34 +1100, Timothy Arceri wrote:
>> From Section 4.4.5 (Uniform and Shader Storage Block Layout
>> Qualifiers) of the OpenGL 4.50 spec:
>>
>>   "The align qualifier makes the start of each block member have a
>>   minimum byte alignment.  It does not affect the internal layout
>>   within each member, which will still follow the std140 or std430
>>   rules. The specified alignment must be a power of 2, or a
>>   compile-time error results.
>>
>>   The actual alignment of a member will be the greater of the
>>   specified align alignment and the standard (e.g., std140) base
>>   alignment for the member's type. The actual offset of a member is
>>   computed as follows: If offset was declared, start with that
>>   offset, otherwise start with the next available offset. If the
>>   resulting offset is not a multiple of the actual alignment,
>>   increase it to the first offset that is a multiple of the actual
>>   alignment. This results in the actual offset the member will have.
>>
>>   When align is applied to an array, it affects only the start of
>>   the array, not the array's internal stride. Both an offset and an
>>   align qualifier can be specified on a declaration.
>>
>>   The align qualifier, when used on a block, has the same effect as
>>   qualifying each member with the same align value as declared on
>>   the block, and gets the same compile-time results and errors as if
>>   this had been done. As described in general earlier, an individual
>>   member can specify its own align, which overrides the block-level
>>   align, but just for that member.
>> ---
>>  src/glsl/ast_to_hir.cpp | 51
>> ++++++++++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 48 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index f5da771..fd05fd7 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -6212,7 +6212,8 @@
>> ast_process_struct_or_iface_block_members(exec_list *instructions,
>>                                            ir_variable_mode var_mode,
>>                                            ast_type_qualifier
>> *layout,
>>                                            unsigned block_stream,
>> -                                          unsigned expl_location)
>> +                                          unsigned expl_location,
>> +                                          unsigned expl_align)
>>  {
>>     unsigned decl_count = 0;
>>     unsigned next_offset = 0;
>> @@ -6466,6 +6467,34 @@
>> ast_process_struct_or_iface_block_members(exec_list *instructions,
>>              }
>>           } else {
>>              fields[i].offset = -1;
>> +         }
>> +
>> +         if (qual->flags.q.explicit_align || expl_align != 0) {
>> +            unsigned offset = fields[i].offset != -1 ?
>> fields[i].offset :
>> +               next_offset;
>> +            if (align == 0 || size == 0) {
>> +               _mesa_glsl_error(&loc, state, "align can only be used
>> with "
>> +                                "std430 and std140 layouts");
>> +            } else if (qual->flags.q.explicit_align) {
>> +               unsigned member_align;
>> +               if (process_qualifier_constant(state, &loc, "align",
>> +                                              qual->align,
>> &member_align)) {
>> +                  if (member_align == 0 ||
> 
> I have modified ubo-block-align-zero.vert piglit test to set the align
> qualifier only to block members and tested with align = 0. The shader
> compiles on NVIDIA proprietary driver.
> 
> According to the spec, the specified alignment must be a power of 2.
> However align = 0 could have different interpretations (for example,
> when applied to a block member, it would mean to ignore block's align
> value). As I am not sure about this case...
> 
> Have you checked if align = 0 is invalid?

I looked at the ARB_enhanced_layouts spec, and it doesn't provide any
real guidance.  My gut tells me that align=0 is not valid because the
spec doesn't say what it means.  Either way, I have submitted a spec bug:

https://www.khronos.org/bugzilla/show_bug.cgi?id=1461

Does glslang allow layout(align=0)?  AMD's closed-source driver?

> Sam
> 
>> +                      member_align & (member_align - 1)) {
>> +                     _mesa_glsl_error(&loc, state, "align layout
>> qualifier "
>> +                                      "in not a power of 2");
>> +                  } else {
>> +                     fields[i].offset = glsl_align(offset,
>> member_align);
>> +                     next_offset = glsl_align(fields[i].offset +
>> size, align);
>> +                  }
>> +               }
>> +            } else {
>> +               fields[i].offset = glsl_align(offset, expl_align);
>> +               next_offset = glsl_align(fields[i].offset + size,
>> align);
>> +            }
>> +         }
>> +
>> +         if (!qual->flags.q.explicit_offset) {
>>              if (align != 0 && size != 0)
>>                 next_offset = glsl_align(next_offset + size, align);
>>           }
>> @@ -6590,7 +6619,8 @@ ast_struct_specifier::hir(exec_list
>> *instructions,
>>                                                  ir_var_auto,
>>                                                  layout,
>>                                                  0, /* for interface
>> only */
>> -                                                expl_location);
>> +                                                expl_location,
>> +                                                0 /* for interface
>> only */);
>>  
>>     validate_identifier(this->name, loc, state);
>>  
>> @@ -6771,6 +6801,20 @@ ast_interface_block::hir(exec_list
>> *instructions,
>>        }
>>     }
>>  
>> +   unsigned expl_align = 0;
>> +   if (layout.flags.q.explicit_align) {
>> +      if (!process_qualifier_constant(state, &loc, "align",
>> +                                      layout.align, &expl_align)) {
>> +         return NULL;
>> +      } else {
>> +         if (expl_align == 0 || expl_align & (expl_align - 1)) {
> 
> Same as before.
> 
> Sam
> 
>> +            _mesa_glsl_error(&loc, state, "align layout qualifier in
>> not a "
>> +                             "power of 2.");
>> +            return NULL;
>> +         }
>> +      }
>> +   }
>> +
>>     unsigned int num_variables =
>>        ast_process_struct_or_iface_block_members(&declared_variables,
>>                                                  state,
>> @@ -6782,7 +6826,8 @@ ast_interface_block::hir(exec_list
>> *instructions,
>>                                                  var_mode,
>>                                                  &this->layout,
>>                                                  qual_stream,
>> -                                                expl_location);
>> +                                                expl_location,
>> +                                                expl_align);
>>  
>>     state->struct_specifier_depth--;
>>  
>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to