On 16/09/15 09:46, Jordan Justen wrote:
> On 2015-09-10 06:35:44, Iago Toral Quiroga wrote:
>> From: Samuel Iglesias Gonsalvez <sigles...@igalia.com>
>>
>> v2:
>> - Get interface packing information from interface's type, not the variable
>> type.
>> - Simplify is_std430 condition in emit_access() for readability (Jordan)
>> - Add a commment explaing why array of three-component vector case is
>> different
>
> Lines a bit long.
>
OK, I will fix it.
>> in std430 than the rest of cases.
>> - Add calls to std430_array_stride().
>>
>> Signed-off-by: Samuel Iglesias Gonsalvez <sigles...@igalia.com>
>> ---
>> src/glsl/lower_ubo_reference.cpp | 102
>> ++++++++++++++++++++++++++++++---------
>> 1 file changed, 78 insertions(+), 24 deletions(-)
>>
>> diff --git a/src/glsl/lower_ubo_reference.cpp
>> b/src/glsl/lower_ubo_reference.cpp
>> index 8694383..7e45a26 100644
>> --- a/src/glsl/lower_ubo_reference.cpp
>> +++ b/src/glsl/lower_ubo_reference.cpp
>> @@ -147,7 +147,8 @@ public:
>> ir_rvalue **offset,
>> unsigned *const_offset,
>> bool *row_major,
>> - int *matrix_columns);
>> + int *matrix_columns,
>> + unsigned packing);
>> ir_expression *ubo_load(const struct glsl_type *type,
>> ir_rvalue *offset);
>> ir_call *ssbo_load(const struct glsl_type *type,
>> @@ -164,7 +165,7 @@ public:
>> void emit_access(bool is_write, ir_dereference *deref,
>> ir_variable *base_offset, unsigned int deref_offset,
>> bool row_major, int matrix_columns,
>> - unsigned write_mask);
>> + bool is_std430, unsigned write_mask);
>>
>> ir_visitor_status visit_enter(class ir_expression *);
>> ir_expression *calculate_ssbo_unsized_array_length(ir_expression *expr);
>> @@ -176,7 +177,8 @@ public:
>> ir_variable *);
>> ir_expression *emit_ssbo_get_buffer_size();
>>
>> - unsigned calculate_unsized_array_stride(ir_dereference *deref);
>> + unsigned calculate_unsized_array_stride(ir_dereference *deref,
>> + unsigned packing);
>>
>> void *mem_ctx;
>> struct gl_shader *shader;
>> @@ -257,7 +259,8 @@
>> lower_ubo_reference_visitor::setup_for_load_or_store(ir_variable *var,
>> ir_rvalue **offset,
>> unsigned *const_offset,
>> bool *row_major,
>> - int *matrix_columns)
>> + int *matrix_columns,
>> + unsigned packing)
>> {
>> /* Determine the name of the interface block */
>> ir_rvalue *nonconst_block_index;
>> @@ -343,8 +346,15 @@
>> lower_ubo_reference_visitor::setup_for_load_or_store(ir_variable *var,
>> const bool array_row_major =
>> is_dereferenced_thing_row_major(deref_array);
>>
>> - array_stride = deref_array->type->std140_size(array_row_major);
>> - array_stride = glsl_align(array_stride, 16);
>> + /* 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;
>> @@ -380,7 +390,12 @@
>> lower_ubo_reference_visitor::setup_for_load_or_store(ir_variable *var,
>>
>> ralloc_free(field_deref);
>>
>> - unsigned field_align =
>> type->std140_base_alignment(field_row_major);
>> + 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);
>>
>> @@ -388,7 +403,10 @@
>> lower_ubo_reference_visitor::setup_for_load_or_store(ir_variable *var,
>> deref_record->field) == 0)
>> break;
>>
>> - intra_struct_offset += type->std140_size(field_row_major);
>> + 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:
>> @@ -437,13 +455,15 @@ lower_ubo_reference_visitor::handle_rvalue(ir_rvalue
>> **rvalue)
>> unsigned const_offset;
>> bool row_major;
>> int matrix_columns;
>> + unsigned packing = var->get_interface_type()->interface_packing;
>>
>> /* Compute the offset to the start if the dereference as well as other
>> * information we need to configure the write
>> */
>> setup_for_load_or_store(var, deref,
>> &offset, &const_offset,
>> - &row_major, &matrix_columns);
>> + &row_major, &matrix_columns,
>> + packing);
>> assert(offset);
>>
>> /* Now that we've calculated the offset to the start of the
>> @@ -463,7 +483,7 @@ lower_ubo_reference_visitor::handle_rvalue(ir_rvalue
>> **rvalue)
>>
>> deref = new(mem_ctx) ir_dereference_variable(load_var);
>> emit_access(false, deref, load_offset, const_offset,
>> - row_major, matrix_columns, 0);
>> + row_major, matrix_columns, false, 0);
>> *rvalue = deref;
>>
>> progress = true;
>> @@ -581,6 +601,7 @@ lower_ubo_reference_visitor::emit_access(bool is_write,
>> unsigned int deref_offset,
>> bool row_major,
>> int matrix_columns,
>> + bool is_std430,
>> unsigned write_mask)
>> {
>> if (deref->type->is_record()) {
>> @@ -599,7 +620,7 @@ lower_ubo_reference_visitor::emit_access(bool is_write,
>>
>> emit_access(is_write, field_deref, base_offset,
>> deref_offset + field_offset,
>> - row_major, 1,
>> + row_major, 1, is_std430,
>>
>> writemask_for_size(field_deref->type->vector_elements));
>>
>> field_offset += field->type->std140_size(row_major);
>> @@ -608,7 +629,8 @@ lower_ubo_reference_visitor::emit_access(bool is_write,
>> }
>>
>> if (deref->type->is_array()) {
>> - unsigned array_stride =
>> + unsigned array_stride = is_std430 ?
>> + deref->type->fields.array->std430_array_stride(row_major) :
>> glsl_align(deref->type->fields.array->std140_size(row_major), 16);
>>
>> for (unsigned i = 0; i < deref->type->length; i++) {
>> @@ -618,7 +640,7 @@ lower_ubo_reference_visitor::emit_access(bool is_write,
>> element);
>> emit_access(is_write, element_deref, base_offset,
>> deref_offset + i * array_stride,
>> - row_major, 1,
>> + row_major, 1, is_std430,
>>
>> writemask_for_size(element_deref->type->vector_elements));
>> }
>> return;
>> @@ -637,7 +659,7 @@ lower_ubo_reference_visitor::emit_access(bool is_write,
>> int size_mul = deref->type->is_double() ? 8 : 4;
>> emit_access(is_write, col_deref, base_offset,
>> deref_offset + i * size_mul,
>> - row_major, deref->type->matrix_columns,
>> + row_major, deref->type->matrix_columns, is_std430,
>>
>> writemask_for_size(col_deref->type->vector_elements));
>> } else {
>> /* std140 always rounds the stride of arrays (and matrices) to a
>> @@ -646,9 +668,26 @@ lower_ubo_reference_visitor::emit_access(bool is_write,
>> */
>> int size_mul = (deref->type->is_double() &&
>> deref->type->vector_elements > 2) ? 32 : 16;
>> +
>> + /* std430 doesn't round up to a vec4 */
>> + if (is_std430) {
>> + if (deref->type->vector_elements <= 2) {
>> + size_mul = deref->type->is_double() ?
>> + 8 * deref->type->vector_elements :
>> + 4 * deref->type->vector_elements;
>> + } else {
>> + /* If the member is a three-component vector with
>> components
>> + * consuming N basic machine units, the base alignment is
>> 4N.
>> + * For vec4, base alignment is 4N.
>> + */
>> + size_mul = (deref->type->is_double() &&
>> + deref->type->vector_elements > 2) ? 32 : 16;
>
> Notice size_mul is reassigned the same value here as above?
>
Yes, it is the same value. I put it explicitly together with the comment
to show that it is intended (because vec3 and vec4 have a base alignment
equals to 4*N also in std430).
> Would this work?
>
> int size_mul;
>
> if (is_std430 && deref->type->vector_elements == 2 &&
> !deref->type->is_double()) {
> size_mul = 8;
> } else {
> size_mul = (deref->type->is_double() &&
> deref->type->vector_elements > 2) ? 32 : 16;
> }
>
Yes, it would work. I will do it.
> I'm assuming vector_elements can't be 1. I'm not certain on that, but,
> https://www.opengl.org/wiki/Data_Type_(GLSL)#Matrices seems to agree
> with this.
>
Yeah, we can assume vector_elements can't be 1 as we are processing
matrices here.
Thanks,
Sam
> -Jordan
>
>> + }
>> + }
>> +
>> emit_access(is_write, col_deref, base_offset,
>> deref_offset + i * size_mul,
>> - row_major, deref->type->matrix_columns,
>> + row_major, deref->type->matrix_columns, is_std430,
>>
>> writemask_for_size(col_deref->type->vector_elements));
>> }
>> }
>> @@ -727,13 +766,16 @@
>> lower_ubo_reference_visitor::write_to_memory(ir_dereference *deref,
>> unsigned const_offset;
>> bool row_major;
>> int matrix_columns;
>> + unsigned packing = var->get_interface_type()->interface_packing;
>> + bool is_std430 = packing == GLSL_INTERFACE_PACKING_STD430;
>>
>> /* Compute the offset to the start if the dereference as well as other
>> * information we need to configure the write
>> */
>> setup_for_load_or_store(var, deref,
>> &offset, &const_offset,
>> - &row_major, &matrix_columns);
>> + &row_major, &matrix_columns,
>> + packing);
>> assert(offset);
>>
>> /* Now emit writes from the temporary to memory */
>> @@ -747,7 +789,7 @@
>> lower_ubo_reference_visitor::write_to_memory(ir_dereference *deref,
>>
>> deref = new(mem_ctx) ir_dereference_variable(write_var);
>> emit_access(true, deref, write_offset, const_offset,
>> - row_major, matrix_columns, write_mask);
>> + row_major, matrix_columns, is_std430, write_mask);
>> }
>>
>> ir_visitor_status
>> @@ -830,7 +872,8 @@ lower_ubo_reference_visitor::emit_ssbo_get_buffer_size()
>> }
>>
>> unsigned
>> -lower_ubo_reference_visitor::calculate_unsized_array_stride(ir_dereference
>> *deref)
>> +lower_ubo_reference_visitor::calculate_unsized_array_stride(ir_dereference
>> *deref,
>> + unsigned
>> packing)
>> {
>> unsigned array_stride = 0;
>>
>> @@ -852,8 +895,12 @@
>> lower_ubo_reference_visitor::calculate_unsized_array_stride(ir_dereference
>> *dere
>> const bool array_row_major =
>> is_dereferenced_thing_row_major(deref_var);
>>
>> - array_stride = unsized_array_type->std140_size(array_row_major);
>> - array_stride = glsl_align(array_stride, 16);
>> + if (packing == GLSL_INTERFACE_PACKING_STD430) {
>> + array_stride =
>> unsized_array_type->std430_array_stride(array_row_major);
>> + } else {
>> + array_stride = unsized_array_type->std140_size(array_row_major);
>> + array_stride = glsl_align(array_stride, 16);
>> + }
>> break;
>> }
>> case ir_type_dereference_record:
>> @@ -868,8 +915,13 @@
>> lower_ubo_reference_visitor::calculate_unsized_array_stride(ir_dereference
>> *dere
>>
>> const bool array_row_major =
>> is_dereferenced_thing_row_major(deref_record);
>> - array_stride = unsized_array_type->std140_size(array_row_major);
>> - array_stride = glsl_align(array_stride, 16);
>> +
>> + if (packing == GLSL_INTERFACE_PACKING_STD430) {
>> + array_stride =
>> unsized_array_type->std430_array_stride(array_row_major);
>> + } else {
>> + array_stride = unsized_array_type->std140_size(array_row_major);
>> + array_stride = glsl_align(array_stride, 16);
>> + }
>> break;
>> }
>> default:
>> @@ -889,14 +941,16 @@
>> lower_ubo_reference_visitor::process_ssbo_unsized_array_length(ir_rvalue
>> **rvalu
>> unsigned const_offset;
>> bool row_major;
>> int matrix_columns;
>> - int unsized_array_stride = calculate_unsized_array_stride(deref);
>> + unsigned packing = var->get_interface_type()->interface_packing;
>> + int unsized_array_stride = calculate_unsized_array_stride(deref,
>> packing);
>>
>> /* Compute the offset to the start if the dereference as well as other
>> * information we need to calculate the length.
>> */
>> setup_for_load_or_store(var, deref,
>> &base_offset, &const_offset,
>> - &row_major, &matrix_columns);
>> + &row_major, &matrix_columns,
>> + packing);
>> /* array.length() =
>> * max((buffer_object_size - offset_of_array) / stride_of_array, 0)
>> */
>> --
>> 1.9.1
>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
