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

[Emil Velikov]

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.

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

Thank
Emil