[Mesa-dev] [PATCH v2] isl: remove ffs function that conflicts with system headers

[Jonathan Gray]

On Mon, Apr 25, 2016 at 04:36:44PM +0100, Emil Velikov wrote:
> On 25 April 2016 at 16:07, Jonathan Gray <jsg at jsg.id.au> wrote:
> > On Mon, Apr 25, 2016 at 03:14:57PM +0100, Emil Velikov wrote:
> >> On 23 April 2016 at 15:36, Jason Ekstrand <jason at jlekstrand.net> wrote:
> >> > Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
> >> >
> >> And pushed. Thanks Jason.
> >>
> >> > In general, we should be using mesa compat headers for this sort of thing
> >> > rather than hand-rolling it.
> >> >
> >> Fully agree with you here. Since ffs is c99 and all targets that mesa
> >> support have it, we don't need one here.
> >> .Iirc the GNU one ffsll was another thing.
> >
> > ffs is POSIX not c99 as far as I can tell, and before that 4.2BSD as a
> > thin wrapper around the vax ffs instruction.
> >
> Indeed. Not sure what I was smoking.
> 
> > According to http://linux.die.net/man/3/ffsll
> > "The ffsl() and ffsll() functions are glibc extensions"
> >
> >>
> >> Fwiw we have an implementation in src/mesa/main/imports.* that we can nuke.
> >>
> >> Jonathan can you do the honours ? Feel free to send over a patch that
> >> drops the __builtin_bswap32 workaround and inlines it.
> >
> > Add a bswap32 to src/mesa/main/imports.* as _mesa_bswap32 you mean?
> >
> > src/gallium/auxiliary/util/u_math.h has a util_bswap32 but non gallium
> > drivers shouldn't be using those definitions?
> These are distinct things:
> 
> - Check if Windows builds work fine without the custom ffs
> implementation. If they don't move it to include/posix_strings.h.
> How to check - wire up your github with AppVeyour. The latter can do
> the builds for you.

It isn't in the VC 2015 CRT according to
https://msdn.microsoft.com/en-us/library/634ca0c2.aspx

so I doubt it.

_BitScanForward builtin seems to be as close as it gets.

> 
> - Inline the ifndef __builtin_bswap32 implementation in the i965 code
> (as mentioned by Matt). The compiler will fold/optimise things as
> needed.

Or add a BE32_TO_CPU/betoh32 type macro to src/mesa/main/compiler.h and
use that?

Though that entire block depends on indirect inclusion to set
BYTE_ORDER and needs to be overhauled.