[ccan] [PATCH] maskn & bitops: add new modules

[Cody P Schafer]

>>> +
>>> +unsigned fls_m1_32(uint32_t v) CONST_FUNCTION;
>>> +unsigned clz_32(uint32_t v) CONST_FUNCTION;
>>> +unsigned ctz_32(uint32_t v) CONST_FUNCTION;
>>
>> Shouldn't these be inlines?  At least the builtin versions; seems a
>> bit silly to use the one-instruction versions when available, then
>> wrap then in a function call.
>>
>
> I end up using lto anyhow, so making the header more complicated
> wasn't a priority.
>
>>> +
>>> +#endif
>>> diff --git a/ccan/bitops/test/run-no-builtin.c b/ccan/bitops/test/run-no-builtin.c
>>> new file mode 100644
>>> index 0000000..5871a1f
>>> --- /dev/null
>>> +++ b/ccan/bitops/test/run-no-builtin.c
>>> @@ -0,0 +1,4 @@
>>> +#include <ccan/bitops/bitops.h>
>>> +#undef builtin_clz
>>> +#undef builtin_ctz
>>> +#include "run.h"
>>
>> This seems really fragile.  A rearrangement of what's in the .c versus
>> the .h could easily mean this silently uses the builtin versions, so
>> not testing what you're trying to test.
>
> But I'm not seeing an alternative that lets me avoid doing the
> preprocessor's work it self (by manually copy-pasting the contents of
> run.h). Do you see one?
>

So looking at these 2 comments again:
 - it appears that my test infra won't actually test the non-builtins
at all (because I'm defining builtin_c*z in the .c)
 - so for the tests to work I'll need to move the builtins into the
header anyway, which opens up the way for providing macros/inlines
anyhow :)