[Mesa-dev] [PATCH] gallium/util: Define ffsll on OpenBSD.
Jose Fonseca
jfonseca at vmware.com
Thu Feb 26 05:42:04 PST 2015
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.
Jose
More information about the mesa-dev
mailing list