[Mesa-dev] [PATCH 3/4] glsl: apply align layout qualifier rules to block offsets
Ian Romanick
idr at freedesktop.org
Tue Feb 9 16:56:17 UTC 2016
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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160209/3078bd7a/attachment.sig>
More information about the mesa-dev
mailing list