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 <j...@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 <j...@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 <matts...@gmail.com>
Date: Fri Feb 20 18:46:43 2015 -0800
gallium/util: Use HAVE___BUILTIN_* macros.
Reviewed-by: Eric Anholt <e...@anholt.net>
Reviewed-by: Jose Fonseca <jfons...@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 <j...@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 <j...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
