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

Ian Romanick Fri, 25 Jan 2013 05:43:40 -0800

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.roman...@intel.com>


Signed-off-by: Ian Romanick <ian.d.roman...@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 powerof two, and it wasn't obvious to me that all the uses of this functionmet that requirement. I did this refactor right before sending thisseries out, and it felt a little like the 11th hour to do something thatcould 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);


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

Reply via email to