[Mesa-dev] [PATCH 3/4] glsl: apply align layout qualifier rules to block offsets
Ian Romanick
idr at freedesktop.org
Tue Feb 9 19:46:09 UTC 2016
On 02/09/2016 08:56 AM, Ian Romanick wrote:
> 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?
According to https://www.khronos.org/bugzilla/show_bug.cgi?id=1461#c1
the Khronos reference compiler does not allow align=0, so, lacking any
contradictory evidence from the spec, I'm happy to not allow it either.
Now I get to figure out how to add a conformance test for this... >:)
>> 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
>
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list