[Mesa-dev] [PATCH 0/2] Move u_bit_scan from gallium to util.

[Brian Paul]

On 06/09/2016 11:02 PM, Mathias Fröhlich wrote:
> Hi,
>
> On Thursday, June 09, 2016 12:35:52 Brian Paul wrote:
>
>  > On 06/09/2016 12:10 PM, Brian Paul wrote:
>
>  > > On 06/09/2016 09:35 AM, Brian Paul wrote:
>
>  > >> On 06/08/2016 11:03 PM, Mathias.Froehlich at gmx.net wrote:
>
>  > >>> From: Mathias Fröhlich <Mathias.Froehlich at gmx.net>
>
>  > >>>
>
>  > >>> Hi all,
>
>  > >>>
>
>  > >>> I just put the two leading patches to move u_bit_scan
>
>  > >>> from gallium to util. The architecture handling of
>
>  > >>> ffs{,ll} is merged with what is available in the
>
>  > >>> mesa/util directory. It compiles and works here and
>
>  > >>> we should get slightly better intrinsic coverage on win32.
>
>  > >>> At least according to what is documented at msdn. But
>
>  > >>> the chance to break one of our other builds or
>
>  > >>> architectures is high with this kind of change.
>
>  > >>> So any hints or testing according that are
>
>  > >>> even more appreciated.
>
>  > >>>
>
>  > >>> The rest of the series posted earlier gets posted
>
>  > >>> again when this part arrived.
>
>  > >>>
>
>  > >>> Please review.
>
>  > >>
>
>  > >> I'm getting a compilation error on MSVC. Will try to debug it today...
>
>  > >
>
>  > > I think we just need to #include "c99_compat.h" in bitscan.h so
> that the
>
>  > > inline keyword gets defined.
>
>  > >
>
>  >
>
>  > And #include bitscan.h in prog_print.c where ffs() is used but not found.
>
> Thanks for testing!
>
> But once you say this - this basically means that I have moved the ffs
>
> definitions out of imports.h. So everybody having relied on finding ffs{,ll}
>
> in imports.h may fail now. Well not really fail, it works on linux but
>
> we likely get the libc function then. And all newly introduced
>
> uses of ffs will likely break your MSVC build as most developers
>
> work on linux rather than with MSVC.
>
> So rather than including bitscan.h directly on each file that wants to
>
> play with ffs we can include that into imports.h?
>
> What do you think?

Long term, I'd like to get away from imports.h defining or including a 
bunch of stuff that a lot of .c files don't need.  So I'd prefer to 
include bitscan.h where needed, but I agree with your concerns.

The patch as you have it now is fine.  I tested it with MSVC.

I see there's some _mesa_fsl() functions still in imports.h.  I think 
those should probably go into bitscan.h too.  A follow-on commit would 
be OK.

Reviewed-by: Brian Paul <brianp at vmware.com>
Tested-by: Brian Paul <brianp at vmware.com>