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

Jonathan Gray jsg at jsg.id.au
Mon Mar 16 18:25:53 PDT 2015


On Mon, Mar 16, 2015 at 08:37:28PM +0000, Emil Velikov wrote:
> On 26/02/15 13:49, Jose Fonseca wrote:
> > 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
> > 
> Jonathan
> 
> Seems like this has ended up a longer discussion that anticipated :\
> Can you please confirm if the above works for you ?
> 
> Thanks
> Emil

It looks like that diff was mangled by the mail client and doesn't have
the newline escaped.  It also assumes a ffsll builtin implies a ffs
builtin is present.  So how about the following instead:

diff --git a/src/gallium/auxiliary/util/u_math.h b/src/gallium/auxiliary/util/u_math.h
index 8f62cac..89c63d7 100644
--- a/src/gallium/auxiliary/util/u_math.h
+++ b/src/gallium/auxiliary/util/u_math.h
@@ -383,14 +383,28 @@ unsigned ffs( unsigned u )
 
    return i;
 }
-#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID)
+#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) || \
+    defined(HAVE___BUILTIN_FFS) 
 #define ffs __builtin_ffs
-#define ffsll __builtin_ffsll
 #endif
 
 #endif /* FFS_DEFINED */
 
 /**
+ * Find first bit set in long long.  Least significant bit is 1.
+ * Return 0 if no bits set.
+ */
+#ifndef FFSLL_DEFINED
+#define FFSLL_DEFINED 1
+
+#if defined(__MINGW32__) || defined(PIPE_OS_ANDROID) || \
+    defined(HAVE___BUILTIN_FFSLL)
+#define ffsll __builtin_ffsll
+#endif
+
+#endif /* FFSLL_DEFINED */
+
+/**
  * Find last bit set in a word.  The least significant bit is 1.
  * Return 0 if no bits are set.
  */


More information about the mesa-dev mailing list