[Mesa-dev] [PATCH v2 19/42] glsl ubo/ssbo: Move common code into lower_buffer_access::setup_buffer_access

Iago Toral itoral at igalia.com
Wed Nov 18 00:00:08 PST 2015


On Tue, 2015-11-17 at 21:54 -0800, Jordan Justen wrote:
> This code will also be usable by the pass to lower shared variables.
> 
> Note, that *const_offset is adjusted by setup_buffer_access so it must
> be initialized before calling setup_buffer_access.
> 
> v2:
>  * Add comment for lower_buffer_access::setup_buffer_access
> 
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> Cc: Iago Toral Quiroga <itoral at igalia.com>
> Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>
> ---
>  src/glsl/lower_buffer_access.cpp | 177 +++++++++++++++++++++++++++++++++++++++
>  src/glsl/lower_buffer_access.h   |   5 ++
>  src/glsl/lower_ubo_reference.cpp | 160 +----------------------------------
>  3 files changed, 185 insertions(+), 157 deletions(-)
> 
> diff --git a/src/glsl/lower_buffer_access.cpp b/src/glsl/lower_buffer_access.cpp
> index b5fe6e3..297ed69 100644
> --- a/src/glsl/lower_buffer_access.cpp
> +++ b/src/glsl/lower_buffer_access.cpp
> @@ -305,4 +305,181 @@ lower_buffer_access::is_dereferenced_thing_row_major(const ir_rvalue *deref)
>     return false;
>  }
>  
> +/**
> + * This function initializes various values that will be used later by
> + * emit_access when actually emitting loads or stores.
> + *
> + * Note: const_offset is an input as well as an output. For UBO and SSBO, the
> + * caller should initialize it to 0 to point to the start of the buffer
> + * object. For compute shader shared variables it will be initialized to the
> + * offset of variable in the shared variable storage block.

I think this is not true, your version changes UBO and SSBO to behave
just like shader variables since you no longer ever update *const_offset
when you find an ir_type_dereference_variable node. Instead, you always
initialize *const_offset to the value of ubo_var->Offset in
setup_for_load_or_store. I think you can just rewrite the note comment
above as:

"const_offset is an input as well as an output, clients must initialize
it to the offset of the variable in the underlying block, and this
function will adjust it by adding the constant offset of the member
being accessed into that variable"

> + */
> +void
> +lower_buffer_access::setup_buffer_access(void *mem_ctx,
> +                                         ir_variable *var,
> +                                         ir_rvalue *deref,
> +                                         ir_rvalue **offset,
> +                                         unsigned *const_offset,
> +                                         bool *row_major,
> +                                         int *matrix_columns,
> +                                         unsigned packing)
> +{
> +   *offset = new(mem_ctx) ir_constant(0u);
> +   *row_major = is_dereferenced_thing_row_major(deref);
> +   *matrix_columns = 1;
> +
> +   /* Calculate the offset to the start of the region of the UBO
> +    * dereferenced by *rvalue.  This may be a variable offset if an
> +    * array dereference has a variable index.
> +    */
> +   while (deref) {
> +      switch (deref->ir_type) {
> +      case ir_type_dereference_variable: {
> +         deref = NULL;
> +         break;
> +      }
> +
> +      case ir_type_dereference_array: {
> +         ir_dereference_array *deref_array = (ir_dereference_array *) deref;
> +         unsigned array_stride;
> +         if (deref_array->array->type->is_vector()) {
> +            /* We get this when storing or loading a component out of a vector
> +             * with a non-constant index. This happens for v[i] = f where v is
> +             * a vector (or m[i][j] = f where m is a matrix). If we don't
> +             * lower that here, it gets turned into v = vector_insert(v, i,
> +             * f), which loads the entire vector, modifies one component and
> +             * then write the entire thing back.  That breaks if another
> +             * thread or SIMD channel is modifying the same vector.
> +             */
> +            array_stride = 4;
> +            if (deref_array->array->type->is_double())
> +               array_stride *= 2;
> +         } else if (deref_array->array->type->is_matrix() && *row_major) {
> +            /* When loading a vector out of a row major matrix, the
> +             * step between the columns (vectors) is the size of a
> +             * float, while the step between the rows (elements of a
> +             * vector) is handled below in emit_ubo_loads.
> +             */
> +            array_stride = 4;
> +            if (deref_array->array->type->is_double())
> +               array_stride *= 2;
> +            *matrix_columns = deref_array->array->type->matrix_columns;
> +         } else if (deref_array->type->without_array()->is_interface()) {
> +            /* We're processing an array dereference of an interface instance
> +             * array. The thing being dereferenced *must* be a variable
> +             * dereference because interfaces cannot be embedded in other
> +             * types. In terms of calculating the offsets for the lowering
> +             * pass, we don't care about the array index. All elements of an
> +             * interface instance array will have the same offsets relative to
> +             * the base of the block that backs them.
> +             */
> +            deref = deref_array->array->as_dereference();
> +            break;
> +         } else {
> +            /* Whether or not the field is row-major (because it might be a
> +             * bvec2 or something) does not affect the array itself. We need
> +             * to know whether an array element in its entirety is row-major.
> +             */
> +            const bool array_row_major =
> +               is_dereferenced_thing_row_major(deref_array);
> +
> +            /* The array type will give the correct interface packing
> +             * information
> +             */
> +            if (packing == GLSL_INTERFACE_PACKING_STD430) {
> +               array_stride = deref_array->type->std430_array_stride(array_row_major);
> +            } else {
> +               array_stride = deref_array->type->std140_size(array_row_major);
> +               array_stride = glsl_align(array_stride, 16);
> +            }
> +         }
> +
> +         ir_rvalue *array_index = deref_array->array_index;
> +         if (array_index->type->base_type == GLSL_TYPE_INT)
> +            array_index = i2u(array_index);
> +
> +         ir_constant *const_index =
> +            array_index->constant_expression_value(NULL);
> +         if (const_index) {
> +            *const_offset += array_stride * const_index->value.u[0];
> +         } else {
> +            *offset = add(*offset,
> +                          mul(array_index,
> +                              new(mem_ctx) ir_constant(array_stride)));
> +         }
> +         deref = deref_array->array->as_dereference();
> +         break;
> +      }
> +
> +      case ir_type_dereference_record: {
> +         ir_dereference_record *deref_record = (ir_dereference_record *) deref;
> +         const glsl_type *struct_type = deref_record->record->type;
> +         unsigned intra_struct_offset = 0;
> +
> +         for (unsigned int i = 0; i < struct_type->length; i++) {
> +            const glsl_type *type = struct_type->fields.structure[i].type;
> +
> +            ir_dereference_record *field_deref = new(mem_ctx)
> +               ir_dereference_record(deref_record->record,
> +                                     struct_type->fields.structure[i].name);
> +            const bool field_row_major =
> +               is_dereferenced_thing_row_major(field_deref);
> +
> +            ralloc_free(field_deref);
> +
> +            unsigned field_align = 0;
> +
> +            if (packing == GLSL_INTERFACE_PACKING_STD430)
> +               field_align = type->std430_base_alignment(field_row_major);
> +            else
> +               field_align = type->std140_base_alignment(field_row_major);
> +
> +            intra_struct_offset = glsl_align(intra_struct_offset, field_align);
> +
> +            if (strcmp(struct_type->fields.structure[i].name,
> +                       deref_record->field) == 0)
> +               break;
> +
> +            if (packing == GLSL_INTERFACE_PACKING_STD430)
> +               intra_struct_offset += type->std430_size(field_row_major);
> +            else
> +               intra_struct_offset += type->std140_size(field_row_major);
> +
> +            /* If the field just examined was itself a structure, apply rule
> +             * #9:
> +             *
> +             *     "The structure may have padding at the end; the base offset
> +             *     of the member following the sub-structure is rounded up to
> +             *     the next multiple of the base alignment of the structure."
> +             */
> +            if (type->without_array()->is_record()) {
> +               intra_struct_offset = glsl_align(intra_struct_offset,
> +                                                field_align);
> +
> +            }
> +         }
> +
> +         *const_offset += intra_struct_offset;
> +         deref = deref_record->record->as_dereference();
> +         break;
> +      }
> +
> +      case ir_type_swizzle: {
> +         ir_swizzle *deref_swizzle = (ir_swizzle *) deref;
> +
> +         assert(deref_swizzle->mask.num_components == 1);
> +
> +         *const_offset += deref_swizzle->mask.x * sizeof(int);
> +         deref = deref_swizzle->val->as_dereference();
> +         break;
> +      }
> +
> +      default:
> +         assert(!"not reached");
> +         deref = NULL;
> +         break;
> +      }
> +   }
> +}
> +
>  } /* namespace lower_buffer_access */
> diff --git a/src/glsl/lower_buffer_access.h b/src/glsl/lower_buffer_access.h
> index b21ea28..f8e1070 100644
> --- a/src/glsl/lower_buffer_access.h
> +++ b/src/glsl/lower_buffer_access.h
> @@ -50,6 +50,11 @@ public:
>                      unsigned int packing, unsigned int write_mask);
>  
>     bool is_dereferenced_thing_row_major(const ir_rvalue *deref);
> +
> +   void setup_buffer_access(void *mem_ctx, ir_variable *var, ir_rvalue *deref,
> +                            ir_rvalue **offset, unsigned *const_offset,
> +                            bool *row_major, int *matrix_columns,
> +                            unsigned packing);
>  };
>  
>  } /* namespace lower_buffer_access */
> diff --git a/src/glsl/lower_ubo_reference.cpp b/src/glsl/lower_ubo_reference.cpp
> index ad7a522..5082da8 100644
> --- a/src/glsl/lower_ubo_reference.cpp
> +++ b/src/glsl/lower_ubo_reference.cpp
> @@ -287,164 +287,10 @@ lower_ubo_reference_visitor::setup_for_load_or_store(ir_variable *var,
>  
>     assert(this->uniform_block);
>  
> -   *offset = new(mem_ctx) ir_constant(0u);
> -   *const_offset = 0;
> -   *row_major = is_dereferenced_thing_row_major(deref);
> -   *matrix_columns = 1;
> -
> -   /* Calculate the offset to the start of the region of the UBO
> -    * dereferenced by *rvalue.  This may be a variable offset if an
> -    * array dereference has a variable index.
> -    */
> -   while (deref) {
> -      switch (deref->ir_type) {
> -      case ir_type_dereference_variable: {
> -         *const_offset += ubo_var->Offset;
> -         deref = NULL;
> -         break;
> -      }
> -
> -      case ir_type_dereference_array: {
> -         ir_dereference_array *deref_array = (ir_dereference_array *) deref;
> -         unsigned array_stride;
> -         if (deref_array->array->type->is_vector()) {
> -            /* We get this when storing or loading a component out of a vector
> -             * with a non-constant index. This happens for v[i] = f where v is
> -             * a vector (or m[i][j] = f where m is a matrix). If we don't
> -             * lower that here, it gets turned into v = vector_insert(v, i,
> -             * f), which loads the entire vector, modifies one component and
> -             * then write the entire thing back.  That breaks if another
> -             * thread or SIMD channel is modifying the same vector.
> -             */
> -            array_stride = 4;
> -            if (deref_array->array->type->is_double())
> -               array_stride *= 2;
> -         } else if (deref_array->array->type->is_matrix() && *row_major) {
> -            /* When loading a vector out of a row major matrix, the
> -             * step between the columns (vectors) is the size of a
> -             * float, while the step between the rows (elements of a
> -             * vector) is handled below in emit_ubo_loads.
> -             */
> -            array_stride = 4;
> -            if (deref_array->array->type->is_double())
> -               array_stride *= 2;
> -            *matrix_columns = deref_array->array->type->matrix_columns;
> -         } else if (deref_array->type->without_array()->is_interface()) {
> -            /* We're processing an array dereference of an interface instance
> -             * array. The thing being dereferenced *must* be a variable
> -             * dereference because interfaces cannot be embedded in other
> -             * types. In terms of calculating the offsets for the lowering
> -             * pass, we don't care about the array index. All elements of an
> -             * interface instance array will have the same offsets relative to
> -             * the base of the block that backs them.
> -             */
> -            deref = deref_array->array->as_dereference();
> -            break;
> -         } else {
> -            /* Whether or not the field is row-major (because it might be a
> -             * bvec2 or something) does not affect the array itself. We need
> -             * to know whether an array element in its entirety is row-major.
> -             */
> -            const bool array_row_major =
> -               is_dereferenced_thing_row_major(deref_array);
> -
> -            /* The array type will give the correct interface packing
> -             * information
> -             */
> -            if (packing == GLSL_INTERFACE_PACKING_STD430) {
> -               array_stride = deref_array->type->std430_array_stride(array_row_major);
> -            } else {
> -               array_stride = deref_array->type->std140_size(array_row_major);
> -               array_stride = glsl_align(array_stride, 16);
> -            }
> -         }
> -
> -         ir_rvalue *array_index = deref_array->array_index;
> -         if (array_index->type->base_type == GLSL_TYPE_INT)
> -            array_index = i2u(array_index);
> -
> -         ir_constant *const_index =
> -            array_index->constant_expression_value(NULL);
> -         if (const_index) {
> -            *const_offset += array_stride * const_index->value.u[0];
> -         } else {
> -            *offset = add(*offset,
> -                          mul(array_index,
> -                              new(mem_ctx) ir_constant(array_stride)));
> -         }
> -         deref = deref_array->array->as_dereference();
> -         break;
> -      }
> -
> -      case ir_type_dereference_record: {
> -         ir_dereference_record *deref_record = (ir_dereference_record *) deref;
> -         const glsl_type *struct_type = deref_record->record->type;
> -         unsigned intra_struct_offset = 0;
> -
> -         for (unsigned int i = 0; i < struct_type->length; i++) {
> -            const glsl_type *type = struct_type->fields.structure[i].type;
> -
> -            ir_dereference_record *field_deref = new(mem_ctx)
> -               ir_dereference_record(deref_record->record,
> -                                     struct_type->fields.structure[i].name);
> -            const bool field_row_major =
> -               is_dereferenced_thing_row_major(field_deref);
> -
> -            ralloc_free(field_deref);
> -
> -            unsigned field_align = 0;
> -
> -            if (packing == GLSL_INTERFACE_PACKING_STD430)
> -               field_align = type->std430_base_alignment(field_row_major);
> -            else
> -               field_align = type->std140_base_alignment(field_row_major);
> -
> -            intra_struct_offset = glsl_align(intra_struct_offset, field_align);
> -
> -            if (strcmp(struct_type->fields.structure[i].name,
> -                       deref_record->field) == 0)
> -               break;
> -
> -            if (packing == GLSL_INTERFACE_PACKING_STD430)
> -               intra_struct_offset += type->std430_size(field_row_major);
> -            else
> -               intra_struct_offset += type->std140_size(field_row_major);
> -
> -            /* If the field just examined was itself a structure, apply rule
> -             * #9:
> -             *
> -             *     "The structure may have padding at the end; the base offset
> -             *     of the member following the sub-structure is rounded up to
> -             *     the next multiple of the base alignment of the structure."
> -             */
> -            if (type->without_array()->is_record()) {
> -               intra_struct_offset = glsl_align(intra_struct_offset,
> -                                                field_align);
> -
> -            }
> -         }
> -
> -         *const_offset += intra_struct_offset;
> -         deref = deref_record->record->as_dereference();
> -         break;
> -      }
> -
> -      case ir_type_swizzle: {
> -         ir_swizzle *deref_swizzle = (ir_swizzle *) deref;
> +   *const_offset = ubo_var->Offset;
>  
> -         assert(deref_swizzle->mask.num_components == 1);
> -
> -         *const_offset += deref_swizzle->mask.x * sizeof(int);
> -         deref = deref_swizzle->val->as_dereference();
> -         break;
> -      }
> -
> -      default:
> -         assert(!"not reached");
> -         deref = NULL;
> -         break;
> -      }
> -   }
> +   setup_buffer_access(mem_ctx, var, deref, offset, const_offset, row_major,
> +                       matrix_columns, packing);
>  }
>  
>  void




More information about the mesa-dev mailing list