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
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
