On Thu, Jun 25, 2015 at 4:16 PM, Anuj Phogat <anuj.pho...@gmail.com> wrote:
> On Thu, Jun 25, 2015 at 3:22 PM, Nanley Chery <nanleych...@gmail.com> wrote:
>> How about if I create a patch which puts the greater than 0 check into
>> is_power_of_two()?
>>
>> (value > 0) && (value & (value - 1)) == 0);
>>
> Not a bad idea except that you'll need to fix conditions at all other places
> which expect the current behavior. I'll leave it up to you. FYI gallium also
> have a helper function util_is_power_of_two() which returns true for 0.
>
I'll avoid putting the condition into the function for now and follow
your suggestion.
>> Thanks,
>> Nanley
>>
>> On Wed, Jun 24, 2015 at 3:22 PM, Anuj Phogat <anuj.pho...@gmail.com> wrote:
>>> On Mon, Jun 22, 2015 at 4:02 PM, Nanley Chery <nanleych...@gmail.com> wrote:
>>>> From: Nanley Chery <nanley.g.ch...@intel.com>
>>>>
>>>> ALIGN and ROUND_DOWN_TO both require that the alignment value passed
>>>> into the macro be a power of two in the comments. Using software assertions
>>>> verifies this to be the case.
>>>>
>>>> v2: use static inline functions instead of gcc-specific statement
>>>> expressions.
>>>>
>>>> Signed-off-by: Nanley Chery <nanley.g.ch...@intel.com>
>>>> ---
>>>> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 2 +-
>>>> src/mesa/main/macros.h | 16 +++++++++++++---
>>>> 2 files changed, 14 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>>> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>>> index 59081ea..1a57784 100644
>>>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>>> @@ -134,7 +134,7 @@ fs_visitor::nir_setup_outputs(nir_shader *shader)
>>>> : var->type->vector_elements;
>>>>
>>>> if (stage == MESA_SHADER_VERTEX) {
>>>> - for (int i = 0; i < ALIGN(type_size(var->type), 4) / 4; i++) {
>>>> + for (unsigned int i = 0; i < ALIGN(type_size(var->type), 4) / 4;
>>>> i++) {
>>>> int output = var->data.location + i;
>>>> this->outputs[output] = offset(reg, 4 * i);
>>>> this->output_components[output] = vector_elements;
>>>> diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
>>>> index 0608650..4a640ad 100644
>>>> --- a/src/mesa/main/macros.h
>>>> +++ b/src/mesa/main/macros.h
>>>> @@ -684,7 +684,7 @@ minify(unsigned value, unsigned levels)
>>>> * Note that this considers 0 a power of two.
>>>> */
>>>> static inline bool
>>>> -is_power_of_two(unsigned value)
>>>> +is_power_of_two(uintptr_t value)
>>>> {
>>>> return (value & (value - 1)) == 0;
>>>> }
>>>> @@ -700,7 +700,12 @@ is_power_of_two(unsigned value)
>>>> *
>>>> * \sa ROUND_DOWN_TO()
>>>> */
>>>> -#define ALIGN(value, alignment) (((value) + (alignment) - 1) &
>>>> ~((alignment) - 1))
>>>> +static inline uintptr_t
>>>> +ALIGN(uintptr_t value, uintptr_t alignment)
>>>> +{
>>>> + assert(is_power_of_two(alignment));
>>> Also handle the 0 alignment. is_power_of_two() returns true for 0.
>>> Use assert(alignment > 0 && is_power_of_two(alignment))?
>>>> + return (((value) + (alignment) - 1) & ~((alignment) - 1));
>>>> +}
>>>>
>>>> /**
>>>> * Align a value down to an alignment value
>>>> @@ -713,7 +718,12 @@ is_power_of_two(unsigned value)
>>>> *
>>>> * \sa ALIGN()
>>>> */
>>>> -#define ROUND_DOWN_TO(value, alignment) ((value) & ~(alignment - 1))
>>>> +static inline uintptr_t
>>>> +ROUND_DOWN_TO(uintptr_t value, uintptr_t alignment)
>>>> +{
>>>> + assert(is_power_of_two(alignment));
>>> Here as well.
>>>> + return ((value) & ~(alignment - 1));
>>>> +}
>>>>
>>>>
>>>> /** Cross product of two 3-element vectors */
>>>> --
>>>> 2.4.2
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>> With above changes and indentation fixes:
>>> Reviewed-by: Anuj Phogat <anuj.pho...@gmail.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
