[Mesa-dev] [PATCH 24/32] glsl: Make the align function available elsewhere in the linker

Ian Romanick idr at freedesktop.org
Fri Jan 25 05:43:04 PST 2013


On 01/24/2013 08:40 PM, Kenneth Graunke wrote:
> On 01/22/2013 12:52 AM, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> ---
>>   src/glsl/glsl_types.cpp          | 12 +++---------
>>   src/glsl/glsl_types.h            |  6 ++++++
>>   src/glsl/link_uniforms.cpp       | 14 ++++----------
>>   src/glsl/lower_ubo_reference.cpp | 19 +++++++------------
>>   4 files changed, 20 insertions(+), 31 deletions(-)
>>
>> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
>> index 0075550..ddd0148 100644
>> --- a/src/glsl/glsl_types.cpp
>> +++ b/src/glsl/glsl_types.cpp
>> @@ -863,12 +863,6 @@ glsl_type::std140_base_alignment(bool row_major)
>> const
>>      return -1;
>>   }
>>
>> -static unsigned
>> -align(unsigned val, unsigned align)
>> -{
>> -   return (val + align - 1) / align * align;
>> -}
>> -
>
> Why not just eliminate this function altogether and use ALIGN() from
> macros.h?  (The implementation is slightly different, but I think it
> should work.)

I thought about that.  The ALIGN macro only works when align is a power 
of two, and it wasn't obvious to me that all the uses of this function 
met that requirement.  I did this refactor right before sending this 
series out, and it felt a little like the 11th hour to do something that 
could have a functional change.

I'd prefer to revisit this after the release.

>>   unsigned
>>   glsl_type::std140_size(bool row_major) const
>>   {
>> @@ -970,11 +964,11 @@ glsl_type::std140_size(bool row_major) const
>>         for (unsigned i = 0; i < this->length; i++) {
>>        const struct glsl_type *field_type =
>> this->fields.structure[i].type;
>>        unsigned align = field_type->std140_base_alignment(row_major);
>> -     size = (size + align - 1) / align * align;
>> +     size = glsl_align(size, align);
>>        size += field_type->std140_size(row_major);
>>         }
>> -      size = align(size,
>> -
>> this->fields.structure[0].type->std140_base_alignment(row_major));
>> +      size = glsl_align(size,
>> +
>> this->fields.structure[0].type->std140_base_alignment(row_major));
>>         return size;
>>      }
>>
>> diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
>> index 8588685..b0db2bf 100644
>> --- a/src/glsl/glsl_types.h
>> +++ b/src/glsl/glsl_types.h
>> @@ -601,6 +601,12 @@ struct glsl_struct_field {
>>      bool row_major;
>>   };
>>
>> +static inline unsigned int
>> +glsl_align(unsigned int a, unsigned int align)
>> +{
>> +   return (a + align - 1) / align * align;
>> +}
>> +
>>   #endif /* __cplusplus */
>>
>>   #endif /* GLSL_TYPES_H */
>> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
>> index 2a1af6b..439b711 100644
>> --- a/src/glsl/link_uniforms.cpp
>> +++ b/src/glsl/link_uniforms.cpp
>> @@ -29,12 +29,6 @@
>>   #include "program/hash_table.h"
>>   #include "program.h"
>>
>> -static inline unsigned int
>> -align(unsigned int a, unsigned int align)
>> -{
>> -   return (a + align - 1) / align * align;
>> -}
>> -
>>   /**
>>    * \file link_uniforms.cpp
>>    * Assign locations for GLSL uniforms.
>> @@ -421,13 +415,13 @@ private:
>>        this->uniforms[id].block_index = this->ubo_block_index;
>>
>>        unsigned alignment = type->std140_base_alignment(ubo_row_major);
>> -     this->ubo_byte_offset = align(this->ubo_byte_offset, alignment);
>> +     this->ubo_byte_offset = glsl_align(this->ubo_byte_offset,
>> alignment);
>>        this->uniforms[id].offset = this->ubo_byte_offset;
>>        this->ubo_byte_offset += type->std140_size(ubo_row_major);
>>
>>        if (type->is_array()) {
>>           this->uniforms[id].array_stride =
>> -           align(type->fields.array->std140_size(ubo_row_major), 16);
>> +           glsl_align(type->fields.array->std140_size(ubo_row_major),
>> 16);
>>        } else {
>>           this->uniforms[id].array_stride = 0;
>>        }
>> @@ -564,7 +558,7 @@ link_assign_uniform_block_offsets(struct gl_shader
>> *shader)
>>        unsigned alignment =
>> type->std140_base_alignment(ubo_var->RowMajor);
>>        unsigned size = type->std140_size(ubo_var->RowMajor);
>>
>> -     offset = align(offset, alignment);
>> +     offset = glsl_align(offset, alignment);
>>        ubo_var->Offset = offset;
>>        offset += size;
>>         }
>> @@ -580,7 +574,7 @@ link_assign_uniform_block_offsets(struct gl_shader
>> *shader)
>>          *      and rounding up to the next multiple of the base
>>          *      alignment required for a vec4."
>>          */
>> -      block->UniformBufferSize = align(offset, 16);
>> +      block->UniformBufferSize = glsl_align(offset, 16);
>>      }
>>   }
>>
>> diff --git a/src/glsl/lower_ubo_reference.cpp
>> b/src/glsl/lower_ubo_reference.cpp
>> index 1d08009..8d13ec1 100644
>> --- a/src/glsl/lower_ubo_reference.cpp
>> +++ b/src/glsl/lower_ubo_reference.cpp
>> @@ -61,12 +61,6 @@ public:
>>      bool progress;
>>   };
>>
>> -static inline unsigned int
>> -align(unsigned int a, unsigned int align)
>> -{
>> -   return (a + align - 1) / align * align;
>> -}
>> -
>>   void
>>   lower_ubo_reference_visitor::handle_rvalue(ir_rvalue **rvalue)
>>   {
>> @@ -113,7 +107,7 @@
>> lower_ubo_reference_visitor::handle_rvalue(ir_rvalue **rvalue)
>>           array_stride = 4;
>>        } else {
>>           array_stride = deref_array->type->std140_size(row_major);
>> -        array_stride = align(array_stride, 16);
>> +        array_stride = glsl_align(array_stride, 16);
>>        }
>>
>>        ir_constant *const_index =
>> deref_array->array_index->as_constant();
>> @@ -138,7 +132,7 @@
>> lower_ubo_reference_visitor::handle_rvalue(ir_rvalue **rvalue)
>>           const glsl_type *type = struct_type->fields.structure[i].type;
>>           unsigned field_align = type->std140_base_alignment(row_major);
>>           max_field_align = MAX2(field_align, max_field_align);
>> -        intra_struct_offset = align(intra_struct_offset, field_align);
>> +        intra_struct_offset = glsl_align(intra_struct_offset,
>> field_align);
>>
>>           if (strcmp(struct_type->fields.structure[i].name,
>>                  deref_record->field) == 0)
>> @@ -146,7 +140,7 @@
>> lower_ubo_reference_visitor::handle_rvalue(ir_rvalue **rvalue)
>>           intra_struct_offset += type->std140_size(row_major);
>>        }
>>
>> -     const_offset = align(const_offset, max_field_align);
>> +     const_offset = glsl_align(const_offset, max_field_align);
>>        const_offset += intra_struct_offset;
>>
>>        deref = deref_record->record->as_dereference();
>> @@ -217,8 +211,8 @@
>> lower_ubo_reference_visitor::emit_ubo_loads(ir_dereference *deref,
>>                              field->name);
>>
>>        field_offset =
>> -        align(field_offset,
>> -          field->type->std140_base_alignment(ubo_var->RowMajor));
>> +        glsl_align(field_offset,
>> +               field->type->std140_base_alignment(ubo_var->RowMajor));
>>
>>        emit_ubo_loads(field_deref, base_offset, deref_offset +
>> field_offset);
>>
>> @@ -229,7 +223,8 @@
>> lower_ubo_reference_visitor::emit_ubo_loads(ir_dereference *deref,
>>
>>      if (deref->type->is_array()) {
>>         unsigned array_stride =
>> -     align(deref->type->fields.array->std140_size(ubo_var->RowMajor),
>> 16);
>> +
>> glsl_align(deref->type->fields.array->std140_size(ubo_var->RowMajor),
>> +            16);
>>
>>         for (unsigned i = 0; i < deref->type->length; i++) {
>>        ir_constant *element = new(mem_ctx) ir_constant(i);
>>



More information about the mesa-dev mailing list