[Mesa-dev] [PATCH v4 (part2) 20/59] glsl: Add parser/compiler support for std430 interface packing qualifier

Jordan Justen jordan.l.justen at intel.com
Fri Aug 28 17:27:28 PDT 2015


On 2015-08-05 01:30:17, Iago Toral Quiroga wrote:
> From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> 
> This commit also adds functions to calculate std430 base alignment and sizes
> 
> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> ---
>  src/glsl/ast.h                   |   1 +
>  src/glsl/ast_to_hir.cpp          |  20 +++++--
>  src/glsl/ast_type.cpp            |   1 +
>  src/glsl/glsl_parser.yy          |   2 +
>  src/glsl/glsl_types.cpp          | 117 +++++++++++++++++++++++++++++++++++++++
>  src/glsl/glsl_types.h            |  15 ++++-
>  src/glsl/link_uniform_blocks.cpp |  15 ++++-
>  src/mesa/main/mtypes.h           |   3 +-
>  8 files changed, 165 insertions(+), 9 deletions(-)
> 
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index d8c6cea..ac3f80f 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -491,6 +491,7 @@ struct ast_type_qualifier {
>          /** \name Layout qualifiers for GL_ARB_uniform_buffer_object */
>          /** \{ */
>           unsigned std140:1;
> +         unsigned std430:1;
>           unsigned shared:1;
>           unsigned packed:1;
>           unsigned column_major:1;
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 6e63d86..2dadb03 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2886,11 +2886,12 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
>         var->data.depth_layout = ir_depth_layout_none;
>  
>     if (qual->flags.q.std140 ||
> +       qual->flags.q.std430 ||
>         qual->flags.q.packed ||
>         qual->flags.q.shared) {
>        _mesa_glsl_error(loc, state,
> -                       "uniform block layout qualifiers std140, packed, and "
> -                       "shared can only be applied to uniform blocks, not "
> +                       "uniform block layout qualifiers std140, std430, packed, "
> +                       "and shared can only be applied to uniform blocks, not "
>                         "members");
>     }
>  
> @@ -5655,12 +5656,14 @@ ast_process_structure_or_interface_block(exec_list *instructions,
>           const struct ast_type_qualifier *const qual =
>              & decl_list->type->qualifier;
>           if (qual->flags.q.std140 ||
> +             qual->flags.q.std430 ||
>               qual->flags.q.packed ||
>               qual->flags.q.shared) {
>              _mesa_glsl_error(&loc, state,
>                               "uniform/shader storage block layout qualifiers "
> -                             "std140, packed, and shared can only be applied "
> -                             "to uniform/shader storage blocks, not members");
> +                             "std140, std430, packed, and shared can only be "
> +                             "applied to uniform/shader storage blocks, not "
> +                             "members");
>           }
>  
>           if (qual->flags.q.constant) {
> @@ -5872,6 +5875,13 @@ ast_interface_block::hir(exec_list *instructions,
>                         this->block_name);
>     }
>  
> +   if (!this->layout.flags.q.buffer &&
> +       this->layout.flags.q.std430) {
> +      _mesa_glsl_error(&loc, state,
> +                       "std430 storage block layout qualifier is supported "
> +                       "only for shader storage blocks");
> +   }
> +
>     /* The ast_interface_block has a list of ast_declarator_lists.  We
>      * need to turn those into ir_variables with an association
>      * with this uniform block.
> @@ -5881,6 +5891,8 @@ ast_interface_block::hir(exec_list *instructions,
>        packing = GLSL_INTERFACE_PACKING_SHARED;
>     } else if (this->layout.flags.q.packed) {
>        packing = GLSL_INTERFACE_PACKING_PACKED;
> +   } else if (this->layout.flags.q.std430) {
> +      packing = GLSL_INTERFACE_PACKING_STD430;
>     } else {
>        /* The default layout is std140.
>         */
> diff --git a/src/glsl/ast_type.cpp b/src/glsl/ast_type.cpp
> index a4671e2..6e69ba2 100644
> --- a/src/glsl/ast_type.cpp
> +++ b/src/glsl/ast_type.cpp
> @@ -123,6 +123,7 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
>     ubo_layout_mask.flags.q.std140 = 1;
>     ubo_layout_mask.flags.q.packed = 1;
>     ubo_layout_mask.flags.q.shared = 1;
> +   ubo_layout_mask.flags.q.std430 = 1;
>  
>     ast_type_qualifier ubo_binding_mask;
>     ubo_binding_mask.flags.i = 0;
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index 2b0c8bd..e1c0f3d 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -1197,6 +1197,8 @@ layout_qualifier_id:
>              $$.flags.q.std140 = 1;
>           } else if (match_layout_qualifier($1, "shared", state) == 0) {
>              $$.flags.q.shared = 1;
> +         } else if (match_layout_qualifier($1, "std430", state) == 0) {
> +            $$.flags.q.std430 = 1;
>           } else if (match_layout_qualifier($1, "column_major", state) == 0) {
>              $$.flags.q.column_major = 1;
>           /* "row_major" is a reserved word in GLSL 1.30+. Its token is parsed
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 755618a..d29769a 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -1357,6 +1357,123 @@ glsl_type::std140_size(bool row_major) const
>     return -1;
>  }
>  
> +unsigned
> +glsl_type::std430_base_alignment(bool row_major) const
> +{
> +   unsigned base_alignment = 0;
> +   /* When using the "std430" storage layout, shader storage
> +    * blocks will be laid out in buffer storage identically to uniform and
> +    * shader storage blocks using the "std140" layout, except that the base
> +    * alignment of arrays of scalars and vectors in rule (4) and of structures
> +    * in rule (9) are not rounded up a multiple of the base alignment of a vec4.
> +    */
> +
> +   /* (1) If the member is a scalar consuming <N> basic machine units, the
> +    *     base alignment is <N>.
> +    *
> +    * (2) If the member is a two- or four-component vector with components
> +    *     consuming <N> basic machine units, the base alignment is 2<N> or
> +    *     4<N>, respectively.
> +    *
> +    * (3) If the member is a three-component vector with components consuming
> +    *     <N> basic machine units, the base alignment is 4<N>.
> +    */
> +   if (this->is_array()) {
> +      base_alignment = this->fields.array->std430_base_alignment(row_major);
> +   } else {
> +      /* For the rest of cases, use std140_base_alignment() */
> +      base_alignment = this->std140_base_alignment(row_major);
> +   }
> +   return base_alignment;
> +}
> +
> +unsigned
> +glsl_type::std430_size(bool row_major) const

I think you should move these two new glsl_type functions into a
separate, earlier commit.

For the parse/link portions of this patch:
Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

> +{
> +   unsigned N = is_double() ? 8 : 4;
> +
> +   /* (1) If the member is a scalar consuming <N> basic machine units, the
> +    *     base alignment is <N>.
> +    *
> +    * (2) If the member is a two- or four-component vector with components
> +    *     consuming <N> basic machine units, the base alignment is 2<N> or
> +    *     4<N>, respectively.
> +    *
> +    * (3) If the member is a three-component vector with components consuming
> +    *     <N> basic machine units, the base alignment is 4<N>.
> +    */
> +   if (this->is_scalar() || this->is_vector()) {
> +      if (this->vector_elements > 2)
> +         return glsl_align(this->vector_elements * N, 16);
> +      else
> +         return this->vector_elements * N;
> +   }
> +
> +   if (this->without_array()->is_matrix()) {
> +      const struct glsl_type *element_type;
> +      const struct glsl_type *vec_type;
> +      unsigned int array_len;
> +
> +      if (this->is_array()) {
> +        element_type = this->fields.array;
> +        array_len = this->length;
> +      } else {
> +        element_type = this;
> +        array_len = 1;
> +      }
> +
> +      if (row_major) {
> +         vec_type = get_instance(element_type->base_type,
> +                                 element_type->matrix_columns, 1);
> +
> +        array_len *= element_type->vector_elements;
> +      } else {
> +        vec_type = get_instance(element_type->base_type,
> +                                element_type->vector_elements, 1);
> +        array_len *= element_type->matrix_columns;
> +      }
> +      const glsl_type *array_type = glsl_type::get_array_instance(vec_type,
> +                                                                 array_len);
> +
> +      return array_type->std430_size(false);
> +   }
> +
> +   /* When using the "std430" storage layout, shader storage
> +    * blocks will be laid out in buffer storage identically to uniform and
> +    * shader storage blocks using the "std140" layout, except that the base
> +    * alignment of arrays of scalars and vectors in rule (4) and of structures
> +    * in rule (9) are not rounded up a multiple of the base alignment of a vec4.
> +    */
> +   if (this->is_array())
> +         return this->length * this->fields.array->std430_size(row_major);

Hmm, you quoted the extension spec, but the 4.30 spec has: "When using
the std430 storage layout, shader storage blocks will be laid out in
buffer storage identically to uniform and shader storage blocks using
the std140 layout, except that the base alignment and stride of arrays
of scalars and vectors in rule 4 and of structures in rule 9 are not
rounded up a multiple of the base alignment of a vec4 ."

Notably, it mentions stride. Wouldn't this be wrong for a vec3 array,
where the stride should be 12 bytes?

I wonder if we'll need to employ some of idr's std140 tricks to work
out all the kinks in std430.

http://www.paranormal-entertainment.com/idr/blog/posts/2014-10-08T13:28:09Z-Stochastic_Search-Based_Testing_for_Uniform_Block_Layouts/

-Jordan

> +
> +   if (this->is_record()) {
> +      unsigned size = 0;
> +      unsigned max_align = 0;
> +
> +      for (unsigned i = 0; i < this->length; i++) {
> +         bool field_row_major = row_major;
> +         const enum glsl_matrix_layout matrix_layout =
> +            glsl_matrix_layout(this->fields.structure[i].matrix_layout);
> +         if (matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR) {
> +            field_row_major = true;
> +         } else if (matrix_layout == GLSL_MATRIX_LAYOUT_COLUMN_MAJOR) {
> +            field_row_major = false;
> +         }
> +
> +         const struct glsl_type *field_type = this->fields.structure[i].type;
> +         unsigned align = field_type->std430_base_alignment(field_row_major);
> +         size = glsl_align(size, align);
> +         size += field_type->std430_size(field_row_major);
> +
> +         max_align = MAX2(align, max_align);
> +      }
> +      size = glsl_align(size, max_align);
> +      return size;
> +   }
> +   /* For the rest of cases, return std140_size(). */
> +   return this->std140_size(row_major);
> +}
>  
>  unsigned
>  glsl_type::count_attribute_slots() const
> diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
> index e7c73da..d994eb5 100644
> --- a/src/glsl/glsl_types.h
> +++ b/src/glsl/glsl_types.h
> @@ -77,7 +77,8 @@ enum glsl_sampler_dim {
>  enum glsl_interface_packing {
>     GLSL_INTERFACE_PACKING_STD140,
>     GLSL_INTERFACE_PACKING_SHARED,
> -   GLSL_INTERFACE_PACKING_PACKED
> +   GLSL_INTERFACE_PACKING_PACKED,
> +   GLSL_INTERFACE_PACKING_STD430
>  };
>  
>  enum glsl_matrix_layout {
> @@ -326,6 +327,18 @@ struct glsl_type {
>     unsigned std140_size(bool row_major) const;
>  
>     /**
> +    * Alignment in bytes of the start of this type in a std430 shader
> +    * storage block.
> +    */
> +   unsigned std430_base_alignment(bool row_major) const;
> +
> +   /** Size in bytes of this type in a std430 shader storage block.
> +    *
> +    * Note that this is not GL_BUFFER_SIZE
> +    */
> +   unsigned std430_size(bool row_major) const;
> +
> +   /**
>      * \brief Can this type be implicitly converted to another?
>      *
>      * \return True if the types are identical or if this type can be converted
> diff --git a/src/glsl/link_uniform_blocks.cpp b/src/glsl/link_uniform_blocks.cpp
> index 4df39e2..c891d03 100644
> --- a/src/glsl/link_uniform_blocks.cpp
> +++ b/src/glsl/link_uniform_blocks.cpp
> @@ -119,8 +119,16 @@ private:
>           v->IndexName = v->Name;
>        }
>  
> -      const unsigned alignment = type->std140_base_alignment(v->RowMajor);
> -      unsigned size = type->std140_size(v->RowMajor);
> +      unsigned alignment = 0;
> +      unsigned size = 0;
> +
> +      if (v->Type->interface_packing == GLSL_INTERFACE_PACKING_STD430) {
> +         alignment = type->std430_base_alignment(v->RowMajor);
> +         size = type->std430_size(v->RowMajor);
> +      } else {
> +         alignment = type->std140_base_alignment(v->RowMajor);
> +         size = type->std140_size(v->RowMajor);
> +      }
>  
>        this->offset = glsl_align(this->offset, alignment);
>        v->Offset = this->offset;
> @@ -255,7 +263,8 @@ link_uniform_blocks(void *mem_ctx,
>                   == unsigned(ubo_packing_shared));
>     STATIC_ASSERT(unsigned(GLSL_INTERFACE_PACKING_PACKED)
>                   == unsigned(ubo_packing_packed));
> -
> +   STATIC_ASSERT(unsigned(GLSL_INTERFACE_PACKING_STD430)
> +                 == unsigned(ubo_packing_std430));
>  
>     hash_table_foreach (block_hash, entry) {
>        const struct link_uniform_block_active *const b =
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index a252bf4..544950a 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2657,7 +2657,8 @@ enum gl_uniform_block_packing
>  {
>     ubo_packing_std140,
>     ubo_packing_shared,
> -   ubo_packing_packed
> +   ubo_packing_packed,
> +   ubo_packing_std430
>  };
>  
>  
> -- 
> 1.9.1
> 


More information about the mesa-dev mailing list