[Mesa-dev] [PATCH 3/4] glsl: apply align layout qualifier rules to block offsets
Samuel Iglesias Gonsálvez
siglesias at igalia.com
Tue Feb 9 14:06:19 UTC 2016
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?
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--;
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160209/b54fac0d/attachment.sig>
More information about the mesa-dev
mailing list