On 2015-11-18 00:00:08, Iago Toral wrote: > 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.jus...@intel.com> > > Cc: Samuel Iglesias Gonsalvez <sigles...@igalia.com> > > Cc: Iago Toral Quiroga <ito...@igalia.com> > > Reviewed-by: Iago Toral Quiroga <ito...@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"
Yeah, that sounds good. I'll use your version. Thanks, -Jordan > > > + */ > > +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 > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev