[Mesa-dev] [PATCH] gallium/util: Define ffsll on OpenBSD.

[Jose Fonseca]

On 26/02/15 13:42, Jose Fonseca wrote:
> On 26/02/15 03:55, Jonathan Gray wrote:
>> On Wed, Feb 25, 2015 at 07:09:26PM -0800, Matt Turner wrote:
>>> On Wed, Feb 25, 2015 at 7:03 PM, Jonathan Gray <jsg at jsg.id.au> wrote:
>>>> On Wed, Feb 25, 2015 at 06:53:14PM -0800, Matt Turner wrote:
>>>>> On Wed, Feb 25, 2015 at 5:37 PM, Jonathan Gray <jsg at jsg.id.au> wrote:
>>>>>> If it isn't going to be configure checks could someone merge the
>>>>>> original patch in this thread?
>>>>>
>>>>> I committed
>>>>>
>>>>> commit 3492e88090d2d0c0bfbc934963b8772b45fc8880
>>>>> Author: Matt Turner <mattst88 at gmail.com>
>>>>> Date:   Fri Feb 20 18:46:43 2015 -0800
>>>>>
>>>>>      gallium/util: Use HAVE___BUILTIN_* macros.
>>>>>
>>>>>      Reviewed-by: Eric Anholt <eric at anholt.net>
>>>>>      Reviewed-by: Jose Fonseca <jfonseca at vmware.com>
>>>>>
>>>>> which switched over a bunch of preprocessor checks around __builtin*
>>>>> calls to use the macros defined by autotools.
>>>>>
>>>>> So I think cleaning it up to use __builtin_ffs* first #ifdef
>>>>> HAVE___BUILTIN_* can go forward now.
>>>>
>>>> Yes but there is no HAVE_FFSLL for constructs like
>>>>
>>>> #if !defined(HAVE_FFSLL) && defined(HAVE___BUILTIN_FFSLL)
>>>>
>>>> or is it ok to always use the builtin?
>>>
>>> I think the question is whether it's okay to always use the builtin if
>>> it's available (as opposed to libc functions). I think the answer to
>>> that is yes.
>>
>> So in that case how about the following?  Or is it going to break
>> the android scons build?
>>
>>  From cba39ba72115e57d262cb4b099c4e72106f01812 Mon Sep 17 00:00:00 2001
>> From: Jonathan Gray <jsg at jsg.id.au>
>> Date: Thu, 26 Feb 2015 14:46:45 +1100
>> Subject: [PATCH] gallium/util: use ffs* builtins if available
>>
>> Required to build on OpenBSD which doesn't have ffsll in libc.
>>
>> Signed-off-by: Jonathan Gray <jsg at jsg.id.au>
>> ---
>>   src/gallium/auxiliary/util/u_math.h | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/util/u_math.h
>> b/src/gallium/auxiliary/util/u_math.h
>> index b4a65e4..5bc9b97 100644
>> --- a/src/gallium/auxiliary/util/u_math.h
>> +++ b/src/gallium/auxiliary/util/u_math.h
>> @@ -384,9 +384,6 @@ unsigned ffs( unsigned u )
>>
>>      return i;
>>   }
>> -#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID)
>> -#define ffs __builtin_ffs
>> -#define ffsll __builtin_ffsll
>
> Scons does define HAVE___BUILTIN_FFS for mingw.
>
> However `git grep '\<ffs\>` shows ffs is used directly in many other
> places.  So I suspect this change will break them.
>
>>   #endif
>>
>>   #endif /* FFS_DEFINED */
>> @@ -435,7 +432,11 @@ util_last_bit_signed(int i)
>>   static INLINE int
>>   u_bit_scan(unsigned *mask)
>>   {
>> +#if defined(HAVE___BUILTIN_FFS)
>> +   int i = __builtin_ffs(*mask) - 1;
>> +#else
>>      int i = ffs(*mask) - 1;
>> +#endif
>>      *mask &= ~(1 << i);
>>      return i;
>>   }
>> @@ -444,7 +445,11 @@ u_bit_scan(unsigned *mask)
>>   static INLINE int
>>   u_bit_scan64(uint64_t *mask)
>>   {
>> +#if defined(HAVE___BUILTIN_FFSLL)
>> +   int i = __builtin_ffsll(*mask) - 1;
>> +#else
>>      int i = ffsll(*mask) - 1;
>> +#endif
>>      *mask &= ~(1llu << i);
>>      return i;
>>   }
>>
>
>
> I think the right thing long term is to provide ffs and ffsll in
> c99_compat.h or c99_math.h for all platforms.  And let the rest of the
> code just always assume it's available somehow.
>
>
> Otherwise, let's just '#define ffs __builtin_ffs' on OpenBSD too.

In other words, the original patch on this thread

   http://lists.freedesktop.org/archives/mesa-dev/2015-February/076071.html

is the only patch I've seen so far that doesn't break Mingw.

If you rather use HAVE___BUILTIN_FFSLL, then just do

diff --git a/src/gallium/auxiliary/util/u_math.h 
b/src/gallium/auxiliary/util/u_math.h
index 959f76e..d372cfd 100644
--- a/src/gallium/auxiliary/util/u_math.h
+++ b/src/gallium/auxiliary/util/u_math.h
@@ -384,7 +384,7 @@ unsigned ffs( unsigned u )

     return i;
  }
-#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID)
+#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) || 
defined(HAVE___BUILTIN_FFSLL)
  #define ffs __builtin_ffs
  #define ffsll __builtin_ffsll
  #endif



Jose