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

[Kenneth Graunke]

On 01/25/2013 05:43 AM, Ian Romanick wrote:
> 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.

Sounds like a good plan.

--Ken